Fix cycle dependency for repeated field not collected by gc (#3399)

2 files changed, 53 insertions, 19 deletions

diff --git a/php/tests/array_test.php b/php/tests/array_test.php
index e57f0a7e..1a26d72a 100644
--- a/php/tests/array_test.php
+++ b/php/tests/array_test.php
@@ -471,6 +471,18 @@ class RepeatedFieldTest extends PHPUnit_Framework_TestCase
         $sub_m->setA(2);
         $arr[0] = $sub_m;
         $this->assertSame(2, $arr[0]->getA());
+
+        // Test foreach.
+        $arr = new RepeatedField(GPBType::MESSAGE, TestMessage_Sub::class);
+        for ($i = 0; $i < 3; $i++) {
+          $arr[] = new TestMessage_Sub();
+          $arr[$i]->setA($i);
+        }
+        $i = 0;
+        foreach ($arr as $val) {
+          $this->assertSame($i++, $val->getA());
+        }
+        $this->assertSame(3, $i);
     }
 
     #########################################################
@@ -521,23 +533,22 @@ class RepeatedFieldTest extends PHPUnit_Framework_TestCase
     # Test memory leak
     #########################################################
 
-    // COMMENTED OUT BY @bshaffer
-    // @see https://github.com/google/protobuf/pull/3344#issuecomment-315162761
-    // public function testCycleLeak()
-    // {
-    //     $arr = new RepeatedField(GPBType::MESSAGE, TestMessage::class);
-    //     $arr[] = new TestMessage;
-    //     $arr[0]->SetRepeatedRecursive($arr);
-
-    //     // Clean up memory before test.
-    //     gc_collect_cycles();
-    //     $start = memory_get_usage();
-    //     unset($arr);
-
-    //     // Explicitly trigger garbage collection.
-    //     gc_collect_cycles();
-
-    //     $end = memory_get_usage();
-    //     $this->assertLessThan($start, $end);
-    // }
+    public function testCycleLeak()
+    {
+        gc_collect_cycles();
+        $arr = new RepeatedField(GPBType::MESSAGE, TestMessage::class);
+        $arr[] = new TestMessage;
+        $arr[0]->SetRepeatedRecursive($arr);
+
+        // Clean up memory before test.
+        gc_collect_cycles();
+        $start = memory_get_usage();
+        unset($arr);
+
+        // Explicitly trigger garbage collection.
+        gc_collect_cycles();
+
+        $end = memory_get_usage();
+        $this->assertLessThan($start, $end);
+    }
 }
diff --git a/php/tests/map_field_test.php b/php/tests/map_field_test.php
index 120b1bde..cffa2526 100644
--- a/php/tests/map_field_test.php
+++ b/php/tests/map_field_test.php
@@ -417,6 +417,29 @@ class MapFieldTest extends PHPUnit_Framework_TestCase {
         $this->assertSame(1, $arr[0]->getA());
 
         $this->assertEquals(1, count($arr));
+
+        // Test foreach.
+        $arr = new MapField(GPBType::INT32,
+            GPBType::MESSAGE, TestMessage_Sub::class);
+        for ($i = 0; $i < 3; $i++) {
+          $arr[$i] = new TestMessage_Sub();;
+          $arr[$i]->setA($i);
+        }
+        $i = 0;
+        $key_test = [];
+        $value_test = [];
+        foreach ($arr as $key => $val) {
+          $key_test[] = $key;
+          $value_test[] = $val->getA();
+          $i++;
+        }
+        $this->assertTrue(isset($key_test['0']));
+        $this->assertTrue(isset($key_test['1']));
+        $this->assertTrue(isset($key_test['2']));
+        $this->assertTrue(isset($value_test['0']));
+        $this->assertTrue(isset($value_test['1']));
+        $this->assertTrue(isset($value_test['2']));
+        $this->assertSame(3, $i);
     }
 
     #########################################################