Skip to content

Commit fbc8f95

Browse files
committed
Fix awaitCompletion/awaitAfterCancellation not marking Future as used
Cancellation tokens passed to Scope::awaitCompletion() and Scope::awaitAfterCancellation() were never marked RESULT_USED/EXC_CAUGHT, causing spurious "Future was never used" warnings. Early return paths (scope already finished/closed/cancelled) bypassed the marking entirely. Move SET_RESULT_USED + SET_EXC_CAUGHT to immediately after parameter parsing, before any early returns.
1 parent 8e1ef6a commit fbc8f95

File tree

4 files changed

+107
-1
lines changed

4 files changed

+107
-1
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ All notable changes to the Async extension for PHP will be documented in this fi
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [0.6.6] -
9+
10+
### Fixed
11+
- **`Scope::awaitCompletion()` not marking cancellation Future as used**: The cancellation token passed to `awaitCompletion()` was never marked with `RESULT_USED` / `EXC_CAUGHT`, causing a spurious "Future was never used" warning when the Future was destroyed. Additionally, early return paths (scope already finished, closed, or cancelled) skipped the marking entirely. Fixed by setting flags immediately after parameter parsing, before any early returns.
12+
- **`Scope::awaitAfterCancellation()` not marking cancellation Future as used**: Same issue as `awaitCompletion()` — the optional cancellation Future was only marked when the method reached `resume_when`, but early returns bypassed it. Fixed identically.
13+
814
## [0.6.5] -
915

1016
### Changed

scope.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,11 @@ METHOD(awaitCompletion)
273273
Z_PARAM_OBJ_OF_CLASS(cancellation_obj, async_ce_awaitable)
274274
ZEND_PARSE_PARAMETERS_END();
275275

276+
// Mark cancellation token as used immediately, before any early returns
277+
zend_async_event_t *cancellation_event = ZEND_ASYNC_OBJECT_TO_EVENT(cancellation_obj);
278+
ZEND_ASYNC_EVENT_SET_RESULT_USED(cancellation_event);
279+
ZEND_ASYNC_EVENT_SET_EXC_CAUGHT(cancellation_event);
280+
276281
zend_coroutine_t *current_coroutine = ZEND_ASYNC_CURRENT_COROUTINE;
277282
if (UNEXPECTED(current_coroutine == NULL)) {
278283
return;
@@ -320,7 +325,7 @@ METHOD(awaitCompletion)
320325
}
321326

322327
zend_async_resume_when(current_coroutine,
323-
ZEND_ASYNC_OBJECT_TO_EVENT(cancellation_obj),
328+
cancellation_event,
324329
false,
325330
zend_async_waker_callback_cancel,
326331
NULL);
@@ -345,6 +350,13 @@ METHOD(awaitAfterCancellation)
345350
Z_PARAM_OBJ_OR_NULL(cancellation_obj)
346351
ZEND_PARSE_PARAMETERS_END();
347352

353+
// Mark cancellation token as used immediately, before any early returns
354+
if (cancellation_obj != NULL) {
355+
zend_async_event_t *cancellation_event = ZEND_ASYNC_OBJECT_TO_EVENT(cancellation_obj);
356+
ZEND_ASYNC_EVENT_SET_RESULT_USED(cancellation_event);
357+
ZEND_ASYNC_EVENT_SET_EXC_CAUGHT(cancellation_event);
358+
}
359+
348360
zend_coroutine_t *current_coroutine = ZEND_ASYNC_CURRENT_COROUTINE;
349361
if (UNEXPECTED(current_coroutine == NULL)) {
350362
return;
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
Scope: awaitCompletion() marks cancellation Future as used
3+
--FILE--
4+
<?php
5+
6+
use Async\Scope;
7+
use Async\Future;
8+
use Async\FutureState;
9+
use function Async\spawn;
10+
use function Async\await;
11+
use function Async\delay;
12+
13+
echo "start\n";
14+
15+
$scope = Scope::inherit();
16+
$scope->spawn(function() {
17+
delay(10);
18+
});
19+
20+
$state = new FutureState();
21+
$future = new Future($state);
22+
23+
$external = spawn(function() use ($scope, $future) {
24+
$scope->awaitCompletion($future);
25+
echo "completed\n";
26+
});
27+
28+
await($external);
29+
30+
// Destroy the future — must NOT trigger "Future was never used" warning
31+
unset($future, $state);
32+
33+
echo "end\n";
34+
35+
?>
36+
--EXPECT--
37+
start
38+
completed
39+
end
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
Scope: awaitAfterCancellation() marks cancellation Future as used
3+
--FILE--
4+
<?php
5+
6+
use Async\Scope;
7+
use Async\Future;
8+
use Async\FutureState;
9+
use function Async\spawn;
10+
use function Async\await;
11+
use function Async\delay;
12+
use function Async\suspend;
13+
14+
echo "start\n";
15+
16+
$scope = Scope::inherit();
17+
$scope->spawn(function() {
18+
try {
19+
delay(50);
20+
} catch (\Async\OperationCanceledException $e) {
21+
// expected
22+
}
23+
});
24+
25+
// Let coroutine start
26+
suspend();
27+
28+
$scope->cancel();
29+
30+
$state = new FutureState();
31+
$future = new Future($state);
32+
33+
$external = spawn(function() use ($scope, $future) {
34+
$scope->awaitAfterCancellation(null, $future);
35+
echo "completed\n";
36+
});
37+
38+
await($external);
39+
40+
// Destroy the future — must NOT trigger "Future was never used" warning
41+
unset($future, $state);
42+
43+
echo "end\n";
44+
45+
?>
46+
--EXPECT--
47+
start
48+
completed
49+
end

0 commit comments

Comments
 (0)