Merge pull request #133 from clue-labs/legacy-garbage

Avoid garbage references when DNS resolution rejects on legacy PHP <= 5.6

Author: jsor

src/Query/CachingExecutor.php

@@ -52,10 +52,14 @@ function (Message $message) use ($cache, $id, $that) {
                         }
                     );
                 }
-            )->then($resolve, $reject);
+            )->then($resolve, function ($e) use ($reject, &$pending) {
+                $reject($e);
+                $pending = null;
+            });
         }, function ($_, $reject) use (&$pending, $query) {
             $reject(new \RuntimeException('DNS query for ' . $query->name . ' has been cancelled'));
             $pending->cancel();
+            $pending = null;
         });
     }
 

src/Query/CoopExecutor.php

@@ -75,10 +75,11 @@ public function query($nameserver, Query $query)
         $counts =& $this->counts;
         return new Promise(function ($resolve, $reject) use ($promise) {
             $promise->then($resolve, $reject);
-        }, function () use ($promise, $key, $query, &$pending, &$counts) {
+        }, function () use (&$promise, $key, $query, &$pending, &$counts) {
             if (--$counts[$key] < 1) {
                 unset($pending[$key], $counts[$key]);
                 $promise->cancel();
+                $promise = null;
             }
             throw new \RuntimeException('DNS query for ' . $query->name . ' has been cancelled');
         });

tests/FunctionalResolverTest.php

@@ -98,4 +98,74 @@ public function testInvalidResolverDoesNotResolveGoogle()
         $promise = $this->resolver->resolve('google.com');
         $promise->then($this->expectCallableNever(), $this->expectCallableOnce());
     }
+
+    public function testResolveShouldNotCauseGarbageReferencesWhenUsingInvalidNameserver()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        $factory = new Factory();
+        $this->resolver = $factory->create('255.255.255.255', $this->loop);
+
+        gc_collect_cycles();
+
+        $promise = $this->resolver->resolve('google.com');
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testResolveCachedShouldNotCauseGarbageReferencesWhenUsingInvalidNameserver()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        $factory = new Factory();
+        $this->resolver = $factory->createCached('255.255.255.255', $this->loop);
+
+        gc_collect_cycles();
+
+        $promise = $this->resolver->resolve('google.com');
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testCancelResolveShouldNotCauseGarbageReferences()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        $factory = new Factory();
+        $this->resolver = $factory->create('127.0.0.1', $this->loop);
+
+        gc_collect_cycles();
+
+        $promise = $this->resolver->resolve('google.com');
+        $promise->cancel();
+        $promise = null;
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testCancelResolveCachedShouldNotCauseGarbageReferences()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        $factory = new Factory();
+        $this->resolver = $factory->createCached('127.0.0.1', $this->loop);
+
+        gc_collect_cycles();
+
+        $promise = $this->resolver->resolve('google.com');
+        $promise->cancel();
+        $promise = null;
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
 }

tests/Query/CoopExecutorTest.php

@@ -205,4 +205,29 @@ public function testQueryTwiceWillQueryBaseExecutorTwiceIfFirstQueryHasAlreadyBe
 
         $promise2->then(null, $this->expectCallableNever());
     }
+
+    public function testCancelQueryShouldNotCauseGarbageReferences()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        $deferred = new Deferred(function () {
+            throw new \RuntimeException();
+        });
+
+        $base = $this->getMockBuilder('React\Dns\Query\ExecutorInterface')->getMock();
+        $base->expects($this->once())->method('query')->willReturn($deferred->promise());
+        $connector = new CoopExecutor($base);
+
+        gc_collect_cycles();
+
+        $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN);
+
+        $promise = $connector->query('8.8.8.8', $query);
+        $promise->cancel();
+        $promise = null;
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
 }