From 190f1bd0c631c0f9f685aa679e55fa404404cd52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= <jerome@tamarelle.net>
Date: Mon, 13 Apr 2026 13:19:25 +0200
Subject: [PATCH] PHPC-2505: Fix get_gc to not interfere with foreach on BSON
 objects

Remove the custom phongo_std_get_gc handler (which delegated to
zend_std_get_properties) so that the default get_gc behavior is used.
Also replace zend_hash_destroy+FREE_HASHTABLE with zend_hash_release
in PHONGO_GET_PROPERTY_HASH_FREE_PROPS, Document.c, and Manager.c to
correctly decrement the reference count rather than forcibly destroying
a potentially shared hashtable.

Add a test (bug2505-001) verifying that foreach iteration on BSON
objects is consistent before and after gc_collect_cycles().
---
 phongo.c                    | 10 ------
 phongo.h                    |  3 +-
 src/BSON/Document.c         |  3 +-
 src/MongoDB/Manager.c       |  3 +-
 tests/bson/bug2505-001.phpt | 71 +++++++++++++++++++++++++++++++++++++
 5 files changed, 74 insertions(+), 16 deletions(-)
 create mode 100644 tests/bson/bug2505-001.phpt

diff --git a/phongo.c b/phongo.c
index 015efd5f8..b4b181ae4 100644
--- a/phongo.c
+++ b/phongo.c
@@ -155,13 +155,6 @@ static zend_class_entry* phongo_fetch_internal_class(const char* class_name, siz
 	return NULL;
 }
 
-static HashTable* phongo_std_get_gc(zend_object* object, zval** table, int* n)
-{
-	*table = NULL;
-	*n     = 0;
-	return zend_std_get_properties(object);
-}
-
 PHP_MINIT_FUNCTION(mongodb) /* {{{ */
 {
 	bson_mem_vtable_t bson_mem_vtable = {
@@ -197,9 +190,6 @@ PHP_MINIT_FUNCTION(mongodb) /* {{{ */
 	/* Disable cloning by default. Individual classes can opt in if they need to
 	 * support this (e.g. BSON objects). */
 	phongo_std_object_handlers.clone_obj = NULL;
-	/* Ensure that get_gc delegates to zend_std_get_properties directly in case
-	 * our class defines a get_properties handler for debugging purposes. */
-	phongo_std_object_handlers.get_gc = phongo_std_get_gc;
 
 	/* Initialize zend_class_entry dependencies.
 	 *
diff --git a/phongo.h b/phongo.h
index d0655f911..b5d55f1be 100644
--- a/phongo.h
+++ b/phongo.h
@@ -77,8 +77,7 @@ zend_object_handlers* phongo_get_std_object_handlers(void);
 #define PHONGO_GET_PROPERTY_HASH_FREE_PROPS(is_temp, props) \
 	do {                                                    \
 		if (is_temp) {                                      \
-			zend_hash_destroy((props));                     \
-			FREE_HASHTABLE(props);                          \
+			zend_hash_release((props));                     \
 		}                                                   \
 	} while (0)
 
diff --git a/src/BSON/Document.c b/src/BSON/Document.c
index 2188b9d10..04eb45306 100644
--- a/src/BSON/Document.c
+++ b/src/BSON/Document.c
@@ -425,8 +425,7 @@ static void phongo_document_free_object(zend_object* object)
 	}
 
 	if (intern->properties) {
-		zend_hash_destroy(intern->properties);
-		FREE_HASHTABLE(intern->properties);
+		zend_hash_release(intern->properties);
 	}
 }
 
diff --git a/src/MongoDB/Manager.c b/src/MongoDB/Manager.c
index abd18a5f2..89a9f5ccd 100644
--- a/src/MongoDB/Manager.c
+++ b/src/MongoDB/Manager.c
@@ -779,8 +779,7 @@ static void phongo_manager_free_object(zend_object* object)
 	}
 
 	if (intern->subscribers) {
-		zend_hash_destroy(intern->subscribers);
-		FREE_HASHTABLE(intern->subscribers);
+		zend_hash_release(intern->subscribers);
 	}
 }
 
diff --git a/tests/bson/bug2505-001.phpt b/tests/bson/bug2505-001.phpt
new file mode 100644
index 000000000..162a2c06e
--- /dev/null
+++ b/tests/bson/bug2505-001.phpt
@@ -0,0 +1,71 @@
+--TEST--
+PHPC-2505: gc_collect_cycles() may interfere with using foreach to iterate objects
+--FILE--
+<?php
+require_once __DIR__ . "/../utils/basic.inc";
+
+gc_disable();
+
+$tests = [
+    [ 'binary' => new MongoDB\BSON\Binary('foo', MongoDB\BSON\Binary::TYPE_GENERIC) ],
+    [ 'dbpointer' => createDBPointer() ],
+    [ 'decimal128' => new MongoDB\BSON\Decimal128('1234.5678') ],
+    [ 'int64' => new MongoDB\BSON\Int64('9223372036854775807') ],
+    // JavaScript w/ scope may not be necessary (same code path as w/o scope), but we'll test it anyway
+    [ 'javascript' => new MongoDB\BSON\Javascript('function() { return 1; }') ],
+
+    // The context is recreated every time with a different object ID
+    //[ 'javascript_ws' => new MongoDB\BSON\Javascript('function() { return a; }', ['a' => 1]) ],
+
+    // MaxKey and MinKey don't have get_properties or get_gc handlers, but we'll test them anyway
+    [ 'maxkey' => new MongoDB\BSON\MaxKey ],
+    [ 'minkey' => new MongoDB\BSON\MinKey ],
+    [ 'objectid' => new MongoDB\BSON\ObjectId ],
+    [ 'regex' => new MongoDB\BSON\Regex('pattern', 'i') ],
+    [ 'symbol' => createSymbol() ],
+    [ 'timestamp' => new MongoDB\BSON\Timestamp(1234, 5678) ],
+    [ 'utcdatetime' => new MongoDB\BSON\UTCDateTime ],
+];
+
+ob_start();
+foreach ($tests as $test) {
+    echo key($test), "\n";
+    $test = reset($test);
+
+    foreach ($test as $k => $v) {
+        var_dump($k, $v);
+    }
+}
+$buf1 = ob_get_clean();
+if ($buf1 === false) {
+    throw new \AssertionError("Could not flush buffer");
+}
+
+gc_enable();
+gc_collect_cycles();
+
+ob_start();
+foreach ($tests as $test) {
+    echo key($test), "\n";
+    $test = reset($test);
+
+    foreach ($test as $k => $v) {
+        var_dump($k, $v);
+    }
+}
+$buf2 = ob_get_clean();
+if ($buf2 === false) {
+    throw new \AssertionError("Could not flush buffer");
+}
+
+if ($buf1 === $buf2) {
+    echo "OK!\n";
+    exit(0);
+} else {
+    echo("buf1 != buf2: $buf1\n\n IS NOT EQUAL TO \n\n$buf2\n");
+    exit(1);
+}
+
+?>
+--EXPECT--
+OK!