Skip to content

ZipArchive subclass storing its own stream is uncollectable after the GH-17787 fix #23276

Description

@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
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.

Activity

  1. LamentXU123 commented on Aug 14, 2026

    @LamentXU123
    Member

    The bug indeed exists... I didn't spot this when reviewing ur last fix. Thank you.

  2. added a commit that references this issue on Aug 24, 2026
    f49b94e
  3. added a commit that references this issue on Aug 24, 2026
    0c3799a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions