Skip to content

Commit 4541391

Browse files
committed
Clean up garbage references for coroutines
1 parent 603e70b commit 4541391

File tree

2 files changed

+100
-1
lines changed

2 files changed

+100
-1
lines changed

src/functions.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,16 +238,19 @@ function coroutine(callable $function, ...$args): PromiseInterface
238238
$next = function () use ($deferred, $generator, &$next, &$promise) {
239239
try {
240240
if (!$generator->valid()) {
241+
$next = null;
241242
$deferred->resolve($generator->getReturn());
242243
return;
243244
}
244245
} catch (\Throwable $e) {
246+
$next = null;
245247
$deferred->reject($e);
246248
return;
247249
}
248250

249251
$promise = $generator->current();
250252
if (!$promise instanceof PromiseInterface) {
253+
$next = null;
251254
$deferred->reject(new \UnexpectedValueException(
252255
'Expected coroutine to yield ' . PromiseInterface::class . ', but got ' . (is_object($promise) ? get_class($promise) : gettype($promise))
253256
));
@@ -260,7 +263,8 @@ function coroutine(callable $function, ...$args): PromiseInterface
260263
}, function (\Throwable $reason) use ($generator, $next) {
261264
$generator->throw($reason);
262265
$next();
263-
})->then(null, function (\Throwable $reason) use ($deferred) {
266+
})->then(null, function (\Throwable $reason) use ($deferred, &$next) {
267+
$next = null;
264268
$deferred->reject($reason);
265269
});
266270
};

tests/CoroutineTest.php

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,4 +143,99 @@ public function testCoroutineWillCancelAllPendingPromisesWhenFunctionContinuesTo
143143

144144
$promise->then(null, $this->expectCallableOnceWith(new \RuntimeException('Second operation cancelled', 42)));
145145
}
146+
147+
public function testCoroutineShouldNotCreateAnyGarbageReferencesWhenGeneratorReturns()
148+
{
149+
if (class_exists('React\Promise\When')) {
150+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
151+
}
152+
153+
gc_collect_cycles();
154+
gc_collect_cycles();
155+
156+
$promise = coroutine(function () {
157+
if (false) {
158+
yield;
159+
}
160+
return 42;
161+
});
162+
163+
unset($promise);
164+
165+
$this->assertEquals(0, gc_collect_cycles());
166+
}
167+
168+
public function testCoroutineShouldNotCreateAnyGarbageReferencesForPromiseRejectedWithExceptionImmediately()
169+
{
170+
if (class_exists('React\Promise\When')) {
171+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
172+
}
173+
174+
gc_collect_cycles();
175+
176+
$promise = coroutine(function () {
177+
yield new Promise(function () {
178+
throw new \RuntimeException('Failed', 42);
179+
});
180+
});
181+
182+
unset($promise);
183+
184+
$this->assertEquals(0, gc_collect_cycles());
185+
}
186+
187+
public function testCoroutineShouldNotCreateAnyGarbageReferencesForPromiseRejectedWithExceptionOnCancellation()
188+
{
189+
if (class_exists('React\Promise\When')) {
190+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
191+
}
192+
193+
gc_collect_cycles();
194+
195+
$promise = coroutine(function () {
196+
yield new Promise(function () { }, function () {
197+
throw new \RuntimeException('Operation cancelled', 42);
198+
});
199+
});
200+
201+
$promise->cancel();
202+
unset($promise);
203+
204+
$this->assertEquals(0, gc_collect_cycles());
205+
}
206+
207+
public function testCoroutineShouldNotCreateAnyGarbageReferencesWhenGeneratorThrowsBeforeFirstYield()
208+
{
209+
if (class_exists('React\Promise\When')) {
210+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
211+
}
212+
213+
gc_collect_cycles();
214+
215+
$promise = coroutine(function () {
216+
throw new \RuntimeException('Failed', 42);
217+
yield;
218+
});
219+
220+
unset($promise);
221+
222+
$this->assertEquals(0, gc_collect_cycles());
223+
}
224+
225+
public function testCoroutineShouldNotCreateAnyGarbageReferencesWhenGeneratorYieldsInvalidValue()
226+
{
227+
if (class_exists('React\Promise\When')) {
228+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
229+
}
230+
231+
gc_collect_cycles();
232+
233+
$promise = coroutine(function () {
234+
yield 42;
235+
});
236+
237+
unset($promise);
238+
239+
$this->assertEquals(0, gc_collect_cycles());
240+
}
146241
}

0 commit comments

Comments
 (0)