Run housekeeping on GC collect (#10)

.github/workflows/ci.yml

@@ -2,20 +2,22 @@ name: CI
 
 on:
   push:
-    branches: [ master ]
   pull_request:
-    branches: [ master ]
 
 jobs:
   phpunit:
     name: PHPUnit
     runs-on: ubuntu-latest
 
     strategy:
+      fail-fast: false
       matrix:
         php-version:
           - "7.4"
           - "8.0" # testing on PHP 8 with an actual WeakMap ensures that our tests are valid
+          - "8.1"
+          - "8.2"
+          - "8.3"
 
     steps:
       - name: Checkout

src/CycleWithDestructor.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace WeakmapPolyfill;
+
+if (\PHP_MAJOR_VERSION === 7) {
+    final class CycleWithDestructor
+    {
+        private \Closure $destructorFx;
+
+        private \stdClass $cycleRef;
+
+        public function __construct(\Closure $destructorFx)
+        {
+            $this->destructorFx = $destructorFx;
+            $this->cycleRef = new \stdClass();
+            $this->cycleRef->x = $this;
+        }
+
+        public function __destruct()
+        {
+            ($this->destructorFx)();
+        }
+    }
+}

src/WeakMap.php

@@ -2,7 +2,9 @@
 
 declare(strict_types=1);
 
-if (! class_exists('WeakMap')) {
+use WeakmapPolyfill\CycleWithDestructor;
+
+if (\PHP_MAJOR_VERSION === 7) {
     /**
      * A polyfill for the upcoming WeakMap implementation in PHP 8, based on WeakReference in PHP 7.4.
      * The polyfill aims to be 100% compatible with the native WeakMap implementation, feature-wise.
@@ -36,6 +38,11 @@ final class WeakMap implements ArrayAccess, Countable, IteratorAggregate
          */
         private const HOUSEKEEPING_THRESHOLD = 10;
 
+        /**
+         * @var array<int, \WeakReference<static>>|null
+         */
+        private static ?array $housekeepingInstances = null;
+
         /**
          * The number of offset*() calls since the last housekeeping.
          */
@@ -53,6 +60,16 @@ final class WeakMap implements ArrayAccess, Countable, IteratorAggregate
          */
         private array $values = [];
 
+        public function __construct()
+        {
+            $this->setupHousekeepingOnGcRun();
+        }
+
+        public function __destruct()
+        {
+            unset(self::$housekeepingInstances[spl_object_id($this)]);
+        }
+
         public function offsetExists($object) : bool
         {
             $this->housekeeping();
@@ -155,7 +172,7 @@ public function __set($name, $value): void {
 
         // NOTE: The native WeakMap does not implement this method,
         // but does forbid serialization.
-        public function __serialize(): void {
+        public function __serialize(): array {
             throw new Exception("Serialization of 'WeakMap' is not allowed");
         }
 
@@ -191,5 +208,38 @@ private function assertValidKey($key) : void
                 throw new TypeError('WeakMap key must be an object');
             }
         }
+
+        /**
+         * @see Based on https://github.com/php/php-src/pull/13650 PHP GC behaviour.
+         */
+        private function setupHousekeepingOnGcRun() : void
+        {
+            if (self::$housekeepingInstances === null) {
+                self::$housekeepingInstances = [];
+
+                $gcRuns = 0;
+                $setupDestructorFx = static function () use (&$gcRuns, &$setupDestructorFx): void {
+                    $destructorFx = static function () use (&$gcRuns, &$setupDestructorFx): void {
+                        $gcRunsPrev = $gcRuns;
+                        $gcRuns = gc_status()['runs'];
+                        if ($gcRunsPrev !== $gcRuns) { // prevent recursion on shutdown
+                            $setupDestructorFx();
+                        }
+
+                        foreach (self::$housekeepingInstances as $v) {
+                            $map = $v->get();
+                            if ($map !== null) {
+                                $map->housekeeping(true);
+                            }
+                        }
+                    };
+
+                    new CycleWithDestructor($destructorFx);
+                };
+                $setupDestructorFx();
+            }
+
+            self::$housekeepingInstances[spl_object_id($this)] = \WeakReference::create($this);
+        }
     }
 }

tests/WeakMapTest.php

@@ -192,10 +192,6 @@ public function testTraversable() : void
 
     public function testHousekeeping() : void
     {
-        if (version_compare(PHP_VERSION, '8') >= 0) {
-            self::markTestSkipped("This test is internal to the polyfill, and will fail with PHP 8's WeakMap");
-        }
-
         $weakMap = new WeakMap();
 
         $k = new stdClass;
@@ -209,14 +205,88 @@ public function testHousekeeping() : void
         unset($k);
         unset($v);
 
-        for ($i = 0; $i < 99; $i++) {
+        if (\PHP_MAJOR_VERSION < 8) {
+            for ($i = 0; $i < 99; $i++) {
+                self::assertNotNull($r->get());
+                isset($weakMap[$unknownObject]);
+            }
+        }
+
+        self::assertNull($r->get());
+    }
+
+    public function testHousekeepingOnGcRun(?WeakMap $weakMap = null) : void
+    {
+        if ($weakMap === null) {
+            $weakMap = new WeakMap();
+        }
+
+        $k = new stdClass;
+        $v = new stdClass;
+        $r = WeakReference::create($v);
+
+        $weakMap[$k] = $v;
+
+        unset($k);
+        unset($v);
+
+        if (\PHP_MAJOR_VERSION < 8) {
             self::assertNotNull($r->get());
-            isset($weakMap[$unknownObject]);
+            gc_collect_cycles();
         }
 
         self::assertNull($r->get());
     }
 
+    public function testNoInternalCycle() : void
+    {
+        $weakMap = new WeakMap();
+        $rWeakMap = WeakReference::create($weakMap);
+
+        $k = new stdClass;
+        $v = new stdClass;
+        $rK = WeakReference::create($k);
+        $rV = WeakReference::create($v);
+
+        $weakMap[$k] = $v;
+
+        unset($weakMap);
+        unset($k);
+        unset($v);
+
+        self::assertNull($rWeakMap->get());
+        self::assertNull($rK->get());
+        self::assertNull($rV->get());
+    }
+
+    public function testHousekeepingOnGcRunSurvival() : void
+    {
+        $weakMap = new WeakMap();
+
+        $vkPairs = [];
+        for ($i = 100; $i > 0; $i--) {
+            for ($j = 100; $j > 0; $j--) {
+                $k = new stdClass;
+                $v = new stdClass;
+                $weakMap[$k] = $v;
+                $vkPairs[] = [$k, $v];
+            }
+
+            gc_collect_cycles();
+            gc_collect_cycles();
+            gc_collect_cycles();
+        }
+
+        foreach ($vkPairs as [$k, $v]) {
+            if ($weakMap[$k] !== $v) {
+                self::assertSame($v, $weakMap[$k]);
+            }
+        }
+        self::assertSame(count($vkPairs), count($weakMap));
+
+        $this->testHousekeepingOnGcRun($weakMap);
+    }
+
     public function testKeyMustBeObjectToSet() : void
     {
         $weakMap = new WeakMap();