[php-src] Issue #23276: ZipArchive subclass storing its own stream is uncollectable after the GH-17787 fix
[email protected] (eyupcanakman)
| Newsgroups | php.bugs |
|---|---|
| Message-ID | <[email protected]> |
Issue: https://github.com/php/php-src/issues/23276
Author: eyupcanakman
### Description
The fix for GH-17787 pins the owning `ZipArchive` from each open stream via `GC_ADDREF` (`ext/zip/zip_stream.c:267` on PHP-8.4), released when the stream closes. That pin is not reported by `php_zip_get_gc`, so when a `ZipArchive` subclass keeps one of its own streams in a property, the cycle collector counts the native reference as external reachability and the object is not collected until request shutdown.
```php
<?php
class Holder extends ZipArchive {
public $s;
}
$dir = sys_get_temp_dir();
gc_collect_cycles();
$before = memory_get_usage();
for ($i = 0; $i < 500; $i++) {
$z = new Holder();
$z->open("$dir/t$i.zip", ZipArchive::CREATE | ZipArchive::OVERWRITE);
$z->addFromString('a.txt', 'x');
$z->s = $z->getStream('a.txt');
unset($z);
}
gc_collect_cycles();
echo memory_get_usage() - $before, "\n";
```
On a debug build of PHP-8.4 this retains about 750 KB across the 500 iterations (~1.5 KB per instance). Reverting only the three files of the fix (`zip_stream.c`, `php_zip.c`, `php_zip.h`) on the same build drops it to a few hundred bytes. A plain `ZipArchive` whose stream is not stored on the object is collected either way.
### Resulted behaviour
The stream-holding subclass instance is uncollectable until request shutdown.
### Expected behaviour
The cycle collector reclaims it, as it does for the plain `ZipArchive` case.
This looks like an accepted-precedent limitation rather than a novel class. `ext/pdo` `pdo_row` pins its statement via `GC_ADDREF` with `get_gc = NULL` the same way.
A refcount-the-`zip_t` approach that avoids the object pin breaks the truncation fix. `addFromString` hands libzip an object-owned buffer with no ownership transfer, so making the object collectable reintroduces the corruption. A real fix moves buffer ownership, which is a larger change to the extension's ownership model.
### PHP version
PHP-8.4, PHP-8.5, master (all carry the fix)
### Operating system
macOS (arm64)
> Disclosure per CONTRIBUTING: this report was drafted with LLM assistance (Claude). The reproduction and the memory measurements were run and verified on a local debug build.