Skip to content

Commit e14f7bd

Browse files
committed
Work around possible cyclic garbage references in Exception trace
1 parent 86d8608 commit e14f7bd

File tree

3 files changed

+70
-2
lines changed

3 files changed

+70
-2
lines changed

src/DnsConnector.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ function ($_, $reject) use (&$promise, &$resolved, $uri) {
104104

105105
// (try to) cancel pending DNS lookup / connection attempt
106106
if ($promise instanceof CancellablePromiseInterface) {
107+
// overwrite callback arguments for PHP7+ only, so they do not show
108+
// up in the Exception trace and do not cause a possible cyclic reference.
109+
$_ = $reject = null;
110+
107111
$promise->cancel();
108112
$promise = null;
109113
}

tests/DnsConnectorTest.php

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,34 @@ public function testRejectionAfterDnsLookupShouldNotCreateAnyGarbageReferences()
242242
$this->assertEquals(0, gc_collect_cycles());
243243
}
244244

245+
/**
246+
* @requires PHP 7
247+
*/
248+
public function testRejectionAfterDnsLookupShouldNotCreateAnyGarbageReferencesAgain()
249+
{
250+
if (class_exists('React\Promise\When')) {
251+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
252+
}
253+
254+
gc_collect_cycles();
255+
256+
$dns = new Deferred();
257+
$this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($dns->promise());
258+
259+
$tcp = new Deferred();
260+
$dns->promise()->then(function () use ($tcp) {
261+
$tcp->reject(new \RuntimeException('Connection failed'));
262+
});
263+
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($tcp->promise());
264+
265+
$promise = $this->connector->connect('example.com:80');
266+
$dns->resolve('1.2.3.4');
267+
268+
unset($promise, $dns, $tcp);
269+
270+
$this->assertEquals(0, gc_collect_cycles());
271+
}
272+
245273
/**
246274
* @requires PHP 7
247275
*/
@@ -289,7 +317,7 @@ public function testCancelDuringTcpConnectionShouldNotCreateAnyGarbageReferences
289317
$dns->resolve('1.2.3.4');
290318

291319
$promise->cancel();
292-
unset($promise, $dns);
320+
unset($promise, $dns, $tcp);
293321

294322
$this->assertEquals(0, gc_collect_cycles());
295323
}

tests/IntegrationTest.php

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ function ($e) use (&$wait) {
184184
/**
185185
* @requires PHP 7
186186
*/
187-
public function testWaitingForConnectionTimeoutShouldNotCreateAnyGarbageReferences()
187+
public function testWaitingForConnectionTimeoutDuringDnsLookupShouldNotCreateAnyGarbageReferences()
188188
{
189189
if (class_exists('React\Promise\When')) {
190190
$this->markTestSkipped('Not supported on legacy Promise v1 API');
@@ -217,6 +217,42 @@ function ($e) use (&$wait) {
217217
$this->assertEquals(0, gc_collect_cycles());
218218
}
219219

220+
/**
221+
* @requires PHP 7
222+
*/
223+
public function testWaitingForConnectionTimeoutDuringTcpConnectionShouldNotCreateAnyGarbageReferences()
224+
{
225+
if (class_exists('React\Promise\When')) {
226+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
227+
}
228+
229+
$loop = Factory::create();
230+
$connector = new Connector($loop, array('timeout' => 0.000001));
231+
232+
gc_collect_cycles();
233+
234+
$wait = true;
235+
$promise = $connector->connect('8.8.8.8:53')->then(
236+
null,
237+
function ($e) use (&$wait) {
238+
$wait = false;
239+
throw $e;
240+
}
241+
);
242+
243+
// run loop for short period to ensure we detect connection timeout error
244+
Block\sleep(0.01, $loop);
245+
if ($wait) {
246+
Block\sleep(0.2, $loop);
247+
if ($wait) {
248+
$this->fail('Connection attempt did not fail');
249+
}
250+
}
251+
unset($promise);
252+
253+
$this->assertEquals(0, gc_collect_cycles());
254+
}
255+
220256
public function testWaitingForInvalidDnsConnectionShouldNotCreateAnyGarbageReferences()
221257
{
222258
if (class_exists('React\Promise\When')) {

0 commit comments

Comments
 (0)