Skip to content

Commit ed23203

Browse files
WyriHaximusWyriHaximus
authored
Merge pull request #55 from clue-labs/cancellation-v4
[4.x] Consistent cancellation semantics for `async()` and `coroutine()`
2 parents 8f4251c + df3c4a1 commit ed23203

File tree

7 files changed

+91
-121
lines changed

7 files changed

+91
-121
lines changed

‎README.md‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -205,18 +205,20 @@ $promise->then(function (int $bytes) {
205205
});
206206
```
207207

208-
Promises returned by `async()` can be cancelled, and when done any currently and future awaited promise inside that and
209-
any nested fibers with their awaited promises will also be cancelled. As such the following example will only output
210-
`ab` as the [`sleep()`](https://reactphp.org/promise-timer/#sleep) between `a` and `b` is cancelled throwing a timeout
211-
exception that bubbles up through the fibers ultimately to the end user through the [`await()`](#await) on the last line
212-
of the example.
208+
The returned promise is implemented in such a way that it can be cancelled
209+
when it is still pending. Cancelling a pending promise will cancel any awaited
210+
promises inside that fiber or any nested fibers. As such, the following example
211+
will only output `ab` and cancel the pending [`sleep()`](https://reactphp.org/promise-timer/#sleep).
212+
The [`await()`](#await) calls in this example would throw a `RuntimeException`
213+
from the cancelled [`sleep()`](https://reactphp.org/promise-timer/#sleep) call
214+
that bubbles up through the fibers.
213215

214216
```php
215217
$promise = async(static function (): int {
216218
echo 'a';
217219
await(async(static function(): void {
218220
echo 'b';
219-
await(sleep(2));
221+
await(React\Promise\Timer\sleep(2));
220222
echo 'c';
221223
})());
222224
echo 'd';

‎composer.json‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@
3131
"react/promise": "^2.8 || ^1.2.1"
3232
},
3333
"require-dev": {
34-
"phpunit/phpunit": "^9.3",
35-
"react/promise-timer": "^1.8"
34+
"phpunit/phpunit": "^9.3"
3635
},
3736
"autoload": {
3837
"psr-4": {

‎src/FiberMap.php‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,6 @@ public static function cancel(\Fiber $fiber): void
2323
self::$status[\spl_object_id($fiber)] = true;
2424
}
2525

26-
public static function isCancelled(\Fiber $fiber): bool
27-
{
28-
return self::$status[\spl_object_id($fiber)] ?? false;
29-
}
30-
3126
public static function setPromise(\Fiber $fiber, PromiseInterface $promise): void
3227
{
3328
self::$map[\spl_object_id($fiber)] = $promise;
@@ -38,11 +33,6 @@ public static function unsetPromise(\Fiber $fiber, PromiseInterface $promise): v
3833
unset(self::$map[\spl_object_id($fiber)]);
3934
}
4035

41-
public static function has(\Fiber $fiber): bool
42-
{
43-
return array_key_exists(\spl_object_id($fiber), self::$map);
44-
}
45-
4636
public static function getPromise(\Fiber $fiber): ?PromiseInterface
4737
{
4838
return self::$map[\spl_object_id($fiber)] ?? null;

‎src/functions.php‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -148,20 +148,20 @@
148148
* });
149149
* ```
150150
*
151-
* Promises returned by `async()` can be cancelled, and when done any currently
152-
* and future awaited promise inside that and any nested fibers with their
153-
* awaited promises will also be cancelled. As such the following example will
154-
* only output `ab` as the [`sleep()`](https://reactphp.org/promise-timer/#sleep)
155-
* between `a` and `b` is cancelled throwing a timeout exception that bubbles up
156-
* through the fibers ultimately to the end user through the [`await()`](#await)
157-
* on the last line of the example.
151+
* The returned promise is implemented in such a way that it can be cancelled
152+
* when it is still pending. Cancelling a pending promise will cancel any awaited
153+
* promises inside that fiber or any nested fibers. As such, the following example
154+
* will only output `ab` and cancel the pending [`sleep()`](https://reactphp.org/promise-timer/#sleep).
155+
* The [`await()`](#await) calls in this example would throw a `RuntimeException`
156+
* from the cancelled [`sleep()`](https://reactphp.org/promise-timer/#sleep) call
157+
* that bubbles up through the fibers.
158158
*
159159
* ```php
160160
* $promise = async(static function (): int {
161161
* echo 'a';
162162
* await(async(static function(): void {
163163
* echo 'b';
164-
* await(sleep(2));
164+
* await(React\Promise\Timer\sleep(2));
165165
* echo 'c';
166166
* })());
167167
* echo 'd';
@@ -277,10 +277,6 @@ function await(PromiseInterface $promise): mixed
277277
$rejectedThrowable = null;
278278
$lowLevelFiber = \Fiber::getCurrent();
279279

280-
if ($lowLevelFiber !== null && FiberMap::isCancelled($lowLevelFiber) && $promise instanceof CancellablePromiseInterface) {
281-
$promise->cancel();
282-
}
283-
284280
$promise->then(
285281
function (mixed $value) use (&$resolved, &$resolvedValue, &$fiber, $lowLevelFiber, $promise): void {
286282
if ($lowLevelFiber !== null) {
@@ -485,12 +481,10 @@ function coroutine(callable $function, mixed ...$args): PromiseInterface
485481

486482
$promise = null;
487483
$deferred = new Deferred(function () use (&$promise) {
488-
// cancel pending promise(s) as long as generator function keeps yielding
489-
while ($promise instanceof CancellablePromiseInterface) {
490-
$temp = $promise;
491-
$promise = null;
492-
$temp->cancel();
484+
if ($promise instanceof PromiseInterface && \method_exists($promise, 'cancel')) {
485+
$promise->cancel();
493486
}
487+
$promise = null;
494488
});
495489

496490
/** @var callable $next */

‎tests/AsyncTest.php‎

Lines changed: 39 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use function React\Promise\all;
1212
use function React\Promise\reject;
1313
use function React\Promise\resolve;
14-
use function React\Promise\Timer\sleep;
1514

1615
class AsyncTest extends TestCase
1716
{
@@ -187,57 +186,70 @@ public function testAsyncReturnsPromiseThatFulfillsWithValueWhenCallbackReturnsA
187186
$this->assertLessThan(0.12, $time);
188187
}
189188

190-
public function testCancel()
189+
public function testCancelAsyncWillReturnRejectedPromiseWhenCancellingPendingPromiseRejects()
191190
{
192-
self::expectOutputString('a');
193-
$this->expectException(\Exception::class);
194-
$this->expectExceptionMessage('Timer cancelled');
191+
$promise = async(function () {
192+
await(new Promise(function () { }, function () {
193+
throw new \RuntimeException('Operation cancelled');
194+
}));
195+
})();
195196

196-
$promise = async(static function (): int {
197-
echo 'a';
198-
await(sleep(2));
199-
echo 'b';
197+
$promise->cancel();
200198

201-
return time();
199+
$promise->then(null, $this->expectCallableOnceWith(new \RuntimeException('Operation cancelled')));
200+
}
201+
202+
public function testCancelAsyncWillReturnFulfilledPromiseWhenCancellingPendingPromiseRejectsInsideCatchThatReturnsValue()
203+
{
204+
$promise = async(function () {
205+
try {
206+
await(new Promise(function () { }, function () {
207+
throw new \RuntimeException('Operation cancelled');
208+
}));
209+
} catch (\RuntimeException $e) {
210+
return 42;
211+
}
202212
})();
203213

204214
$promise->cancel();
205-
await($promise);
215+
216+
$promise->then($this->expectCallableOnceWith(42));
206217
}
207218

208-
public function testCancelTryCatch()
219+
public function testCancelAsycWillReturnPendigPromiseWhenCancellingFirstPromiseRejectsInsideCatchThatAwaitsSecondPromise()
209220
{
210-
self::expectOutputString('ab');
211-
212-
$promise = async(static function (): int {
213-
echo 'a';
221+
$promise = async(function () {
214222
try {
215-
await(sleep(2));
216-
} catch (\Throwable) {
217-
// No-Op
223+
await(new Promise(function () { }, function () {
224+
throw new \RuntimeException('First operation cancelled');
225+
}));
226+
} catch (\RuntimeException $e) {
227+
await(new Promise(function () { }, function () {
228+
throw new \RuntimeException('Second operation never cancelled');
229+
}));
218230
}
219-
echo 'b';
220-
221-
return time();
222231
})();
223232

224233
$promise->cancel();
225-
await($promise);
234+
235+
$promise->then($this->expectCallableNever(), $this->expectCallableNever());
226236
}
227237

228-
public function testNestedCancel()
238+
public function testCancelAsyncWillCancelNestedAwait()
229239
{
230240
self::expectOutputString('abc');
231-
$this->expectException(\Exception::class);
232-
$this->expectExceptionMessage('Timer cancelled');
241+
$this->expectException(\RuntimeException::class);
242+
$this->expectExceptionMessage('Operation cancelled');
233243

234244
$promise = async(static function (): int {
235245
echo 'a';
236246
await(async(static function(): void {
237247
echo 'b';
238248
await(async(static function(): void {
239249
echo 'c';
240-
await(sleep(2));
250+
await(new Promise(function () { }, function () {
251+
throw new \RuntimeException('Operation cancelled');
252+
}));
241253
echo 'd';
242254
})());
243255
echo 'e';
@@ -250,44 +262,4 @@ public function testNestedCancel()
250262
$promise->cancel();
251263
await($promise);
252264
}
253-
254-
public function testCancelFiberThatCatchesExceptions()
255-
{
256-
self::expectOutputString('ab');
257-
$this->expectException(\Exception::class);
258-
$this->expectExceptionMessage('Timer cancelled');
259-
260-
$promise = async(static function (): int {
261-
echo 'a';
262-
try {
263-
await(sleep(2));
264-
} catch (\Throwable) {
265-
// No-Op
266-
}
267-
echo 'b';
268-
await(sleep(0.1));
269-
echo 'c';
270-
271-
return time();
272-
})();
273-
274-
$promise->cancel();
275-
await($promise);
276-
}
277-
278-
public function testNotAwaitedPromiseWillNotBeCanceled()
279-
{
280-
self::expectOutputString('acb');
281-
282-
async(static function (): int {
283-
echo 'a';
284-
sleep(0.001)->then(static function (): void {
285-
echo 'b';
286-
});
287-
echo 'c';
288-
289-
return time();
290-
})()->cancel();
291-
Loop::run();
292-
}
293265
}

‎tests/AwaitTest.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,9 @@ public function testResolvedPromisesShouldBeDetached(callable $await)
379379
{
380380
$await(async(function () use ($await): int {
381381
$fiber = \Fiber::getCurrent();
382-
$await(React\Promise\Timer\sleep(0.01));
382+
$await(new Promise(function ($resolve) {
383+
Loop::addTimer(0.01, fn() => $resolve(null));
384+
}));
383385
$this->assertNull(React\Async\FiberMap::getPromise($fiber));
384386

385387
return time();

‎tests/CoroutineTest.php‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -106,42 +106,53 @@ public function testCoroutineReturnsRejectedPromiseIfFunctionYieldsInvalidValue(
106106
$promise->then(null, $this->expectCallableOnceWith(new \UnexpectedValueException('Expected coroutine to yield React\Promise\PromiseInterface, but got integer')));
107107
}
108108

109-
110-
public function testCoroutineWillCancelPendingPromiseWhenCallingCancelOnResultingPromise()
109+
public function testCancelCoroutineWillReturnRejectedPromiseWhenCancellingPendingPromiseRejects()
111110
{
112-
$cancelled = 0;
113-
$promise = coroutine(function () use (&$cancelled) {
114-
yield new Promise(function () use (&$cancelled) {
115-
++$cancelled;
111+
$promise = coroutine(function () {
112+
yield new Promise(function () { }, function () {
113+
throw new \RuntimeException('Operation cancelled');
116114
});
117115
});
118116

119117
$promise->cancel();
120118

121-
$this->assertEquals(1, $cancelled);
119+
$promise->then(null, $this->expectCallableOnceWith(new \RuntimeException('Operation cancelled')));
122120
}
123121

124-
public function testCoroutineWillCancelAllPendingPromisesWhenFunctionContinuesToYieldWhenCallingCancelOnResultingPromise()
122+
public function testCancelCoroutineWillReturnFulfilledPromiseWhenCancellingPendingPromiseRejectsInsideCatchThatReturnsValue()
125123
{
126124
$promise = coroutine(function () {
127-
$promise = new Promise(function () { }, function () {
128-
throw new \RuntimeException('Frist operation cancelled', 21);
129-
});
130-
131125
try {
132-
yield $promise;
126+
yield new Promise(function () { }, function () {
127+
throw new \RuntimeException('Operation cancelled');
128+
});
133129
} catch (\RuntimeException $e) {
134-
// ignore exception and continue
130+
return 42;
135131
}
132+
});
136133

137-
yield new Promise(function () { }, function () {
138-
throw new \RuntimeException('Second operation cancelled', 42);
139-
});
134+
$promise->cancel();
135+
136+
$promise->then($this->expectCallableOnceWith(42));
137+
}
138+
139+
public function testCancelCoroutineWillReturnPendigPromiseWhenCancellingFirstPromiseRejectsInsideCatchThatYieldsSecondPromise()
140+
{
141+
$promise = coroutine(function () {
142+
try {
143+
yield new Promise(function () { }, function () {
144+
throw new \RuntimeException('First operation cancelled');
145+
});
146+
} catch (\RuntimeException $e) {
147+
yield new Promise(function () { }, function () {
148+
throw new \RuntimeException('Second operation never cancelled');
149+
});
150+
}
140151
});
141152

142153
$promise->cancel();
143154

144-
$promise->then(null, $this->expectCallableOnceWith(new \RuntimeException('Second operation cancelled', 42)));
155+
$promise->then($this->expectCallableNever(), $this->expectCallableNever());
145156
}
146157

147158
public function testCoroutineShouldNotCreateAnyGarbageReferencesWhenGeneratorReturns()

0 commit comments

Comments
 (0)