Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/Internal/Workflow/ChildWorkflowStub.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
use Temporal\Internal\Transport\Request\ExecuteChildWorkflow;
use Temporal\Internal\Transport\Request\GetChildWorkflowExecution;
use Temporal\Internal\Transport\Request\SignalExternalWorkflow;
use Temporal\Worker\FeatureFlags;
use Temporal\Worker\Transport\Command\RequestInterface;
use Temporal\Workflow;
use Temporal\Workflow\ChildWorkflowOptions;
use Temporal\Workflow\ChildWorkflowStubInterface;
use Temporal\Workflow\ParentClosePolicy;
use Temporal\Workflow\WorkflowExecution;

final class ChildWorkflowStub implements ChildWorkflowStubInterface
Expand Down Expand Up @@ -70,7 +72,10 @@ public function start(... $args): PromiseInterface
$this->header,
);

$this->result = $this->request($this->request);
$cancellable = FeatureFlags::$cancelAbandonedChildWorkflows
|| $this->options->parentClosePolicy !== ParentClosePolicy::Abandon->value;

$this->result = $this->request($this->request, cancellable: $cancellable);

$started = $this->request(new GetChildWorkflowExecution($this->request))
->then(
Expand Down Expand Up @@ -118,12 +123,12 @@ function (WorkflowExecution $execution) use ($name, $args) {
);
}

protected function request(RequestInterface $request): PromiseInterface
protected function request(RequestInterface $request, bool $cancellable = true): PromiseInterface
{
/** @var Workflow\WorkflowContextInterface $context */
$context = Workflow::getCurrentContext();

return $context->request($request);
return $context->request($request, cancellable: $cancellable);
}

private function getOptionArray(): array
Expand Down
9 changes: 7 additions & 2 deletions src/Internal/Workflow/Process/Scope.php
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,9 @@ protected function callSignalOrUpdateHandler(callable $handler, ValuesInterface
}, $values)->catch($this->onException(...));
}

protected function onRequest(RequestInterface $request, PromiseInterface $promise): void
protected function onRequest(RequestInterface $request, PromiseInterface $promise, bool $cancellable = true): void
{
$this->onCancel[++$this->cancelID] = function (?\Throwable $reason = null) use ($request): void {
$this->onCancel[++$this->cancelID] = function (?\Throwable $reason = null) use ($request, $cancellable): void {
$client = $this->context->getClient();
if ($reason instanceof DestructMemorizedInstanceException) {
// memory flush
Expand All @@ -370,6 +370,11 @@ protected function onRequest(RequestInterface $request, PromiseInterface $promis
return;
}

if (!$cancellable) {
// non-cancellable request
return;
}

$client->request(new Cancel($request->getID()), $this->scopeContext);
};

Expand Down
12 changes: 8 additions & 4 deletions src/Internal/Workflow/ScopeContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@

class ScopeContext extends WorkflowContext implements ScopedContextInterface
{
/** @var \Closure(RequestInterface, PromiseInterface, bool $cancellable): void */
private \Closure $onRequest;

private WorkflowContext $parent;
private Scope $scope;
private \Closure $onRequest;
private ?UpdateContext $updateContext = null;

/**
* Creates scope specific context.
*
* @param \Closure(RequestInterface, PromiseInterface, bool $cancellable): void $onRequest
*/
public static function fromWorkflowContext(
WorkflowContext $context,
Expand Down Expand Up @@ -78,11 +82,11 @@ public function request(
);

if (!$waitResponse) {
return $this->parent->request($request, $cancellable, false);
return $this->parent->request($request, cancellable: $cancellable, waitResponse: false);
}

$promise = $this->parent->request($request);
($this->onRequest)($request, $promise);
$promise = $this->parent->request($request, cancellable: $cancellable);
($this->onRequest)($request, $promise, $cancellable);

return new CompletableResult(
$this,
Expand Down
17 changes: 17 additions & 0 deletions src/Worker/FeatureFlags.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,21 @@ final class FeatureFlags
* @since SDK 2.11.0
*/
public static bool $warnOnWorkflowUnfinishedHandlers = true;

/**
* When a parent workflow is canceled, it will also cancel all its Child Workflows, including abandoned ones.
* This behavior is not correct and will be improved by default in the next major SDK version.
*
* To fix the behavior now, set this flag to TRUE. In this case, be aware of the following:
* - If you start an abandoned Child Workflow in the main Workflow scope, it may miss
* the cancellation signal if you await only on the Child Workflow.
* - If you start an abandoned Child Workflow in an async scope {@see Workflow::async()},
* that is later canceled, the Child Workflow will not be affected.
* - You still can cancel abandoned Child Workflows manually by calling {@see WorkflowStubInterface::cancel()}.
*
* @see Workflow\ParentClosePolicy::Abandon
*
* @since SDK 2.16.0
*/
public static bool $cancelAbandonedChildWorkflows = true;
}
2 changes: 2 additions & 0 deletions src/Workflow/WorkflowContextInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ public function registerUpdate(string $name, callable $handler, ?callable $valid
/**
* Exchanges data between worker and host process.
*
* @param bool $waitResponse Determine if the Request requires a Response from RoadRunner.
*
* @internal This is an internal method
*/
public function request(
Expand Down
1 change: 1 addition & 0 deletions tests/Acceptance/App/RuntimeBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public static function init(): void
\ini_set('display_errors', 'stderr');
// Feature flags
FeatureFlags::$workflowDeferredHandlerStart = true;
FeatureFlags::$cancelAbandonedChildWorkflows = false;
}

/**
Expand Down
169 changes: 148 additions & 21 deletions tests/Acceptance/Harness/ChildWorkflow/CancelAbandonTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,102 @@
use Temporal\Client\WorkflowStubInterface;
use Temporal\Exception\Failure\CanceledFailure;
use Temporal\Exception\Failure\ChildWorkflowFailure;
use Temporal\Promise;
use Temporal\Tests\Acceptance\App\Attribute\Stub;
use Temporal\Tests\Acceptance\App\TestCase;
use Temporal\Workflow;
use Temporal\Workflow\CancellationScopeInterface;
use Temporal\Workflow\WorkflowInterface;
use Temporal\Workflow\WorkflowMethod;

class CancelAbandonTest extends TestCase
{
/**
* If an abandoned Child Workflow is started in the main Workflow scope,
* the Child Workflow should not be affected by the cancellation of the parent workflow.
* But need to consider that we can miss the Cancellation signal if awaiting only on the Child Workflow.
* In the {@see MainScopeWorkflow} we use Timer + Child Workflow to ensure we catch the Cancellation signal.
*/
#[Test]
public static function check(
#[Stub('Harness_ChildWorkflow_CancelAbandon')]
public static function childWorkflowInMainScope(
#[Stub('Harness_ChildWorkflow_CancelAbandon_MainScope', args: ['test 42'])]
WorkflowStubInterface $stub,
WorkflowClientInterface $client,
): void {
self::markTestSkipped('To be resolved with https://github.com/temporalio/sdk-php/issues/634');
self::runTestScenario($stub, $client, 'test 42');
}

/**
* If an abandoned Child Workflow is started in an async Scope {@see Workflow::async()} that is later cancelled,
* the Child Workflow should not be affected by the cancellation of the parent workflow.
* Int his case the Scope will throw the CanceledFailure.
* @see InnerScopeCancelWorkflow
*/
#[Test]
public static function childWorkflowInInnerScopeCancel(
#[Stub('Harness_ChildWorkflow_CancelAbandon_InnerScopeCancel', args: ['baz'])]
WorkflowStubInterface $stub,
WorkflowClientInterface $client,
): void {
self::runTestScenario($stub, $client, 'baz');
}

/**
* If an abandoned Child Workflow is started in an async scope {@see Workflow::async()} that
* is later cancelled manually by a Signal to the parent workflow {@see InnerScopeCancelWorkflow::close()},
* the Child Workflow should not be affected by the cancellation of the parent scope.
*/
#[Test]
public static function childWorkflowInClosingInnerScope(
#[Stub('Harness_ChildWorkflow_CancelAbandon_InnerScopeCancel', args: ['foo bar'])]
WorkflowStubInterface $stub,
WorkflowClientInterface $client,
): void {
# Get Child Workflow Stub
$child = self::getChildWorkflowStub($client, $stub);

# Cancel the async scope
/** @see InnerScopeCancelWorkflow::close() */
$stub->signal('close');
# Expect the CanceledFailure in the parent workflow
self::assertSame('cancelled', $stub->getResult(timeout: 5));

# Signal the child workflow to exit
$child->signal('exit');
# No canceled failure in the child workflow
self::assertSame('foo bar', $child->getResult());
}

/**
* Send cancel to the parent workflow and expect the child workflow to be abandoned
* and not cancelled.
*/
private static function runTestScenario(
WorkflowStubInterface $stub,
WorkflowClientInterface $client,
string $result,
): void {
# Get Child Workflow Stub
$child = self::getChildWorkflowStub($client, $stub);

# Cancel the parent workflow
$stub->cancel();
# Expect the CanceledFailure in the parent workflow
self::assertSame('cancelled', $stub->getResult(timeout: 5));

# Signal the child workflow to exit
$child->signal('exit');
# No canceled failure in the child workflow
self::assertSame($result, $child->getResult());
}

/**
* Get Child Workflow Stub
*/
private static function getChildWorkflowStub(
WorkflowClientInterface $client,
WorkflowStubInterface $stub,
): WorkflowStubInterface {
# Find the child workflow execution ID
$deadline = \microtime(true) + 10;
child_id:
Expand All @@ -40,52 +120,99 @@ public static function check(
goto child_id;
}

self::assertNotNull($execution, 'Child workflow execution not found in history');
self::assertNotNull($execution, 'Child Workflow execution not found in the history.');

# Get Child Workflow Stub
$child = $client->newUntypedRunningWorkflowStub(
return $client->newUntypedRunningWorkflowStub(
$execution->getWorkflowId(),
$execution->getRunId(),
'Harness_ChildWorkflow_CancelAbandon_Child',
);

# Cancel the parent workflow
$stub->cancel();
# Expect the CanceledFailure in the parent workflow
self::assertSame('cancelled', $stub->getResult());

# Signal the child workflow to exit
$child->signal('exit');
# No canceled failure in the child workflow
self::assertSame('test 42', $child->getResult());
}
}

#[WorkflowInterface]
class MainWorkflow
class MainScopeWorkflow
{
#[WorkflowMethod('Harness_ChildWorkflow_CancelAbandon')]
public function run()
#[WorkflowMethod('Harness_ChildWorkflow_CancelAbandon_MainScope')]
public function run(string $input)
{
$child = Workflow::newUntypedChildWorkflowStub(
/** @see ChildWorkflow */
$stub = Workflow::newUntypedChildWorkflowStub(
'Harness_ChildWorkflow_CancelAbandon_Child',
Workflow\ChildWorkflowOptions::new()
->withWorkflowRunTimeout('20 seconds')
->withParentClosePolicy(Workflow\ParentClosePolicy::Abandon),
);

yield $child->start('test 42');
yield $stub->start($input);

try {
yield Promise::race([$stub->getResult(), Workflow::timer(5)]);
return 'timer';
} catch (CanceledFailure) {
return 'cancelled';
} catch (ChildWorkflowFailure $failure) {
# Check CanceledFailure
return $failure->getPrevious()::class === CanceledFailure::class
? 'cancelled'
: throw $failure;
} finally {
yield Workflow::asyncDetached(function () {
# We shouldn't complete the Workflow immediately:
# all the commands from the tick must be sent for testing purposes.
yield Workflow::timer(1);
});
}
}
}

#[WorkflowInterface]
class InnerScopeCancelWorkflow
{
private CancellationScopeInterface $scope;

#[WorkflowMethod('Harness_ChildWorkflow_CancelAbandon_InnerScopeCancel')]
public function run(string $input)
{
$this->scope = Workflow::async(static function () use ($input) {
/** @see ChildWorkflow */
$stub = Workflow::newUntypedChildWorkflowStub(
'Harness_ChildWorkflow_CancelAbandon_Child',
Workflow\ChildWorkflowOptions::new()
->withWorkflowRunTimeout('20 seconds')
->withParentClosePolicy(Workflow\ParentClosePolicy::Abandon),
);
yield $stub->start($input);

return yield $stub->getResult('string');
});


try {
return yield $child->getResult();
yield Promise::race([Workflow::timer(5) ,$this->scope]);
return 'timer';
} catch (CanceledFailure) {
return 'cancelled';
} catch (ChildWorkflowFailure $failure) {
# Check CanceledFailure
return $failure->getPrevious()::class === CanceledFailure::class
? 'cancelled'
: throw $failure;
} finally {
yield Workflow::asyncDetached(function () {
# We shouldn't complete the Workflow immediately:
# all the commands from the tick must be sent for testing purposes.
yield Workflow::timer(1);
});
}
}

#[Workflow\SignalMethod('close')]
public function close(): void
{
$this->scope->cancel();
}
}

#[WorkflowInterface]
Expand Down
Loading