Skip to content

Commit d40c5cc

Browse files
committed
Implement bugfix for child-workflow bug in #1138
Resolves #1138 by correcting the bug. As this is a non-backwards-compatible change, the next commit contains a "backport" of sorts to allow selecting the buggy behavior if necessary, to ease migration.
1 parent 7d3b4c4 commit d40c5cc

File tree

2 files changed

+28
-6
lines changed

2 files changed

+28
-6
lines changed

internal/internal_workflow_testsuite_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3158,8 +3158,6 @@ func (s *WorkflowTestSuiteUnitTest) Test_Regression_ExecuteChildWorkflowWithCanc
31583158
})
31593159
s.Run("canceled before child starts", func() {
31603160
// ... and should not start the child (i.e. be canceled) when canceled before it is started.
3161-
check(0, "no err") // but it does not! this is a bug to fix.
3162-
// this should be:
3163-
// check(0, "canceled")
3161+
check(0, "canceled")
31643162
})
31653163
}

internal/workflow.go

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -896,6 +896,14 @@ func (wc *workflowEnvironmentInterceptor) ExecuteChildWorkflow(ctx Context, chil
896896
decodeFutureImpl: mainFuture.(*decodeFutureImpl),
897897
executionFuture: executionFuture.(*futureImpl),
898898
}
899+
900+
// Starting with a canceled context should immediately fail, no need to even try.
901+
if ctx.Err() != nil {
902+
mainSettable.SetError(ctx.Err())
903+
executionSettable.SetError(ctx.Err())
904+
return result
905+
}
906+
899907
workflowOptionsFromCtx := getWorkflowEnvOptions(ctx)
900908
dc := workflowOptionsFromCtx.dataConverter
901909
env := getWorkflowEnvironment(ctx)
@@ -928,6 +936,7 @@ func (wc *workflowEnvironmentInterceptor) ExecuteChildWorkflow(ctx Context, chil
928936

929937
ctxDone, cancellable := ctx.Done().(*channelImpl)
930938
cancellationCallback := &receiveCallback{}
939+
shouldCancelAsync := false
931940
err = getWorkflowEnvironment(ctx).ExecuteChildWorkflow(params, func(r []byte, e error) {
932941
mainSettable.Set(r, e)
933942
if cancellable {
@@ -939,6 +948,11 @@ func (wc *workflowEnvironmentInterceptor) ExecuteChildWorkflow(ctx Context, chil
939948
childWorkflowExecution = &r
940949
}
941950
executionSettable.Set(r, e)
951+
952+
// forward the delayed cancellation if necessary
953+
if shouldCancelAsync && e == nil && !mainFuture.IsReady() {
954+
getWorkflowEnvironment(ctx).RequestCancelChildWorkflow(*options.domain, childWorkflowExecution.ID)
955+
}
942956
})
943957

944958
if err != nil {
@@ -949,9 +963,19 @@ func (wc *workflowEnvironmentInterceptor) ExecuteChildWorkflow(ctx Context, chil
949963

950964
if cancellable {
951965
cancellationCallback.fn = func(v interface{}, more bool) bool {
952-
if ctx.Err() == ErrCanceled && childWorkflowExecution != nil && !mainFuture.IsReady() {
953-
// child workflow started, and ctx cancelled
954-
getWorkflowEnvironment(ctx).RequestCancelChildWorkflow(*options.domain, childWorkflowExecution.ID)
966+
if ctx.Err() == ErrCanceled {
967+
if childWorkflowExecution != nil && !mainFuture.IsReady() {
968+
// child workflow started, and ctx cancelled. forward cancel to the child.
969+
getWorkflowEnvironment(ctx).RequestCancelChildWorkflow(*options.domain, childWorkflowExecution.ID)
970+
} else if childWorkflowExecution == nil {
971+
// decision to start the child has been made, but it has not yet started.
972+
973+
// TODO: ideal, but not strictly necessary for correctness:
974+
// if it's in the same decision, revoke that cancel synchronously.
975+
976+
// if the decision has already gone through: wait for it to be started, and then cancel it.
977+
shouldCancelAsync = true
978+
}
955979
}
956980
return false
957981
}

0 commit comments

Comments
 (0)