Skip to content

Commit 18bfef4

Browse files
authored
fix: propagate cancellation to scopes and awaits registered after cancel (#770)
* fix: propagate cancellation to scopes and awaits registered after cancel Scope cancellation was a one-shot pass over the onCancel handlers: any cancellation-aware resource registered after cancel() had already run was never notified. As a result a nested scope created under an already cancelled parent was born uncancelled, and an await registered in an already cancelled scope blocked forever (the workflow got stuck). Introduce a register-or-fire-immediately helper (addOnCancel) used by the public onCancel(), onAwait() and createScope(); store the cancellation reason so late handlers receive it; and guard onException()/onResult() against a double close pass. This matches the Go/Java/TS SDKs, where cancellation is sticky and observed at registration time. Adds acceptance tests covering nested-scope inheritance, fail-fast await, the public onCancel hook, and a faithful replica of the issue reproduction (each one hangs without the fix). Fixes #769 * test: add assertions for trailed runLocked behavior * feat(workflow): gate cancellation propagation behind a feature flag Make the sticky scope-cancellation behavior opt-in via FeatureFlags::$propagateCancellationToNewScopes (default false = previous behavior). When enabled, a nested scope, await or onCancel handler registered after the surrounding scope was already cancelled is notified immediately instead of being missed. - Scope reads the flag at addOnCancel() and nextPromise(). - Acceptance suite enables it in RuntimeBuilder::init() alongside the other flags. - Add a unit test covering both flag states.
1 parent 9a0f42d commit 18bfef4

7 files changed

Lines changed: 473 additions & 18 deletions

File tree

src/Internal/Workflow/Process/Scope.php

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use Temporal\Internal\Transport\Request\Cancel;
2727
use Temporal\Internal\Workflow\ScopeContext;
2828
use Temporal\Internal\Workflow\WorkflowContext;
29+
use Temporal\Worker\FeatureFlags;
2930
use Temporal\Worker\LoopInterface;
3031
use Temporal\Worker\Transport\Command\RequestInterface;
3132
use Temporal\Workflow;
@@ -87,6 +88,8 @@ class Scope implements CancellationScopeInterface, Destroyable
8788

8889
private bool $detached = false;
8990
private bool $cancelled = false;
91+
private bool $closed = false;
92+
private ?\Throwable $cancelReason = null;
9093

9194
public function __construct(
9295
ServiceContainer $services,
@@ -191,7 +194,7 @@ public function attach(\Generator $generator): self
191194

192195
public function onCancel(callable $then): self
193196
{
194-
$this->onCancel[++$this->cancelID] = $then;
197+
$this->addOnCancel($then);
195198
return $this;
196199
}
197200

@@ -217,6 +220,7 @@ public function cancel(?\Throwable $reason = null): void
217220
}
218221

219222
$this->cancelled = true;
223+
$this->cancelReason = $reason;
220224

221225
foreach ($this->onCancel as $i => $handler) {
222226
$this->makeCurrent();
@@ -280,12 +284,9 @@ public function always(callable $onFulfilledOrRejected): PromiseInterface
280284
*/
281285
public function onAwait(Deferred $deferred): void
282286
{
283-
$this->onCancel[++$this->cancelID] = static function (?\Throwable $e = null) use ($deferred): void {
287+
$cancelID = $this->addOnCancel(static function (?\Throwable $e = null) use ($deferred): void {
284288
$deferred->reject($e ?? new CanceledFailure(''));
285-
};
286-
287-
$cancelID = $this->cancelID;
288-
289+
});
289290

290291
// do not cancel already complete promises
291292
$cleanup = function () use ($cancelID): void {
@@ -329,8 +330,7 @@ protected function createScope(
329330
$scope->layer = $layer;
330331
}
331332

332-
$cancelID = ++$this->cancelID;
333-
$this->onCancel[$cancelID] = $scope->cancel(...);
333+
$cancelID = $this->addOnCancel($scope->cancel(...));
334334

335335
$scope->onClose(
336336
function () use ($cancelID): void {
@@ -371,7 +371,7 @@ protected function callSignalOrUpdateHandler(callable $handler, ValuesInterface
371371

372372
protected function onRequest(RequestInterface $request, PromiseInterface $promise, bool $cancellable = true): void
373373
{
374-
$this->onCancel[++$this->cancelID] = function (?\Throwable $reason = null) use ($request, $cancellable): void {
374+
$cancelID = $this->addOnCancel(function (?\Throwable $reason = null) use ($request, $cancellable): void {
375375
$client = $this->context->getClient();
376376
if ($reason instanceof DestructMemorizedInstanceException) {
377377
// memory flush
@@ -390,9 +390,7 @@ protected function onRequest(RequestInterface $request, PromiseInterface $promis
390390
}
391391

392392
$client->request(new Cancel($request->getID()), $this->scopeContext);
393-
};
394-
395-
$cancelID = $this->cancelID;
393+
}, $cancellable);
396394

397395
// do not cancel already complete promises
398396
$cleanup = function () use ($cancelID): void {
@@ -460,10 +458,27 @@ protected function next(): void
460458
}
461459
}
462460

461+
private function addOnCancel(callable $handler, bool $cancellable = true): int
462+
{
463+
$id = ++$this->cancelID;
464+
465+
if (FeatureFlags::$propagateCancellationToNewScopes && $this->cancelled && $cancellable) {
466+
$this->makeCurrent();
467+
$handler($this->cancelReason);
468+
return $id;
469+
}
470+
471+
$this->onCancel[$id] = $handler;
472+
return $id;
473+
}
474+
463475
private function nextPromise(PromiseInterface $promise): void
464476
{
465477
if ($promise instanceof CancellationScopeInterface && $promise->isCancelled()) {
466-
$this->handleError(new CanceledFailure(''));
478+
$reason = FeatureFlags::$propagateCancellationToNewScopes && $promise instanceof self
479+
? $promise->cancelReason
480+
: null;
481+
$this->handleError($reason ?? new CanceledFailure(''));
467482
return;
468483
}
469484

@@ -525,6 +540,11 @@ private function handleError(\Throwable $e): void
525540

526541
private function onException(\Throwable $e): void
527542
{
543+
if ($this->closed) {
544+
return;
545+
}
546+
547+
$this->closed = true;
528548
$this->deferred->reject($e);
529549

530550
$this->makeCurrent();
@@ -537,6 +557,11 @@ private function onException(\Throwable $e): void
537557

538558
private function onResult(mixed $result): void
539559
{
560+
if ($this->closed) {
561+
return;
562+
}
563+
564+
$this->closed = true;
540565
$this->deferred->resolve($result);
541566

542567
$this->makeCurrent();

src/Worker/FeatureFlags.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,18 @@ final class FeatureFlags
6464
* @since SDK 2.17.0
6565
*/
6666
public static bool $warnOnActivityMethodWithoutAttribute = true;
67+
68+
/**
69+
* Make scope cancellation sticky: a nested scope, an await or an onCancel handler registered
70+
* after the surrounding scope was already cancelled is notified immediately instead of being
71+
* missed. Set to TRUE to enable this behavior.
72+
*
73+
* When FALSE (default), the previous behavior is kept: cancel handlers registered after the
74+
* cancellation are never invoked.
75+
*
76+
* @experimental
77+
* @since SDK 2.18.0
78+
* @link https://github.com/temporalio/sdk-php/issues/769
79+
*/
80+
public static bool $propagateCancellationToNewScopes = false;
6781
}

tests/Acceptance/App/RuntimeBuilder.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ public static function init(): void
8888
FeatureFlags::$workflowDeferredHandlerStart = true;
8989
FeatureFlags::$cancelAbandonedChildWorkflows = false;
9090
FeatureFlags::$warnOnActivityMethodWithoutAttribute = true;
91+
FeatureFlags::$propagateCancellationToNewScopes = true;
9192
}
9293

9394
/**

0 commit comments

Comments
 (0)