PHPC-2505: Fix foreach after GC invocation

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

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)
 

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);
 	}
 }
 

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);
 	}
 }
 

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!