Skip to content

Commit 41a023c

Browse files
committed
Use static progress callback without binding to promise
1 parent 389551e commit 41a023c

File tree

2 files changed

+43
-14
lines changed

2 files changed

+43
-14
lines changed

src/Promise.php

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -164,17 +164,6 @@ private function reject($reason = null)
164164
$this->settle(reject($reason));
165165
}
166166

167-
private function notify($update = null)
168-
{
169-
if (null !== $this->result) {
170-
return;
171-
}
172-
173-
foreach ($this->progressHandlers as $handler) {
174-
$handler($update);
175-
}
176-
}
177-
178167
private function settle(ExtendedPromiseInterface $promise)
179168
{
180169
$promise = $this->unwrap($promise);
@@ -239,16 +228,23 @@ private function call(callable $callback)
239228
if ($args === 0) {
240229
$callback();
241230
} else {
231+
// Store a reference to all progress handlers (will be cleared when settled)
232+
// This way, we can use a static progress callback that is not bound to this promise instance.
233+
// This helps avoiding garbage cycles if the callback creates an Exception.
234+
$progress =& $this->progressHandlers;
235+
242236
$callback(
243237
function ($value = null) {
244238
$this->resolve($value);
245239
},
246240
function ($reason = null) {
247241
$this->reject($reason);
248242
},
249-
function ($update = null) {
250-
$this->notify($update);
251-
}
243+
\Closure::bind(function ($update = null) use (&$progress) {
244+
foreach ($progress as $handler) {
245+
$handler($update);
246+
}
247+
}, null)
252248
);
253249
}
254250
} catch (\Throwable $e) {

tests/PromiseTest.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,39 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfResolverThrowsExceptio
6060
$this->assertSame(0, gc_collect_cycles());
6161
}
6262

63+
/**
64+
* test that checks number of garbage cycles after throwing from a resolver
65+
* that has its arguments explicitly set to null (reassigned arguments only
66+
* show up in the stack trace in PHP 7, so we can't test this on legacy PHP)
67+
*
68+
* @test
69+
* @requires PHP 7
70+
* @link https://3v4l.org/OiDr4
71+
*/
72+
public function shouldRejectWithoutCreatingGarbageCyclesIfResolverThrowsExceptionWithResolveAndRejectUnset()
73+
{
74+
gc_collect_cycles();
75+
$promise = new Promise(function ($resolve, $reject) {
76+
$resolve = $reject = null;
77+
throw new \Exception('foo');
78+
});
79+
unset($promise);
80+
81+
$this->assertSame(0, gc_collect_cycles());
82+
}
83+
84+
/** @test */
85+
public function shouldIgnoreNotifyAfterReject()
86+
{
87+
$promise = new Promise(function () { }, function ($resolve, $reject, $notify) {
88+
$reject(new \Exception('foo'));
89+
$notify(42);
90+
});
91+
92+
$promise->then(null, null, $this->expectCallableNever());
93+
$promise->cancel();
94+
}
95+
6396
/** @test */
6497
public function shouldFulfillIfFullfilledWithSimplePromise()
6598
{

0 commit comments

Comments
 (0)