diff --git a/internal/internal_workflow_testsuite.go b/internal/internal_workflow_testsuite.go index 21b785fc6..43f36e669 100644 --- a/internal/internal_workflow_testsuite.go +++ b/internal/internal_workflow_testsuite.go @@ -2452,13 +2452,26 @@ func (env *testWorkflowEnvironmentImpl) RequestCancelChildWorkflow(_, workflowID func (env *testWorkflowEnvironmentImpl) RequestCancelExternalWorkflow(namespace, workflowID, runID string, callback ResultHandler) { if env.workflowInfo.WorkflowExecution.ID == workflowID { - // cancel current workflow - env.workflowCancelHandler() - // check if current workflow is a child workflow - if env.isChildWorkflow() && env.onChildWorkflowCanceledListener != nil { - env.postCallback(func() { - env.onChildWorkflowCanceledListener(env.workflowInfo) - }, false) + cancelFunc := func() { + env.workflowCancelHandler() + + if env.isChildWorkflow() && env.onChildWorkflowCanceledListener != nil { + env.postCallback(func() { + env.onChildWorkflowCanceledListener(env.workflowInfo) + }, false) + } + } + // The way testWorkflowEnvironment is setup today, we close the child workflow dispatcher before calling + // the workflowCancelHandler. A larger refactor would be needed to handle this similar to non-test code. + // Maybe worth doing when https://github.com/temporalio/go-sdk/issues/50 is tackled. + if sd, ok := env.workflowDef.(*syncWorkflowDefinition); ok && env.isChildWorkflow() { + if !sd.dispatcher.IsClosed() { + sd.dispatcher.NewCoroutine(sd.rootCtx, "cancel-self", true, func(ctx Context) { + cancelFunc() + }) + } + } else { + cancelFunc() } return } else if childHandle, ok := env.runningWorkflows[workflowID]; ok && !childHandle.handled { diff --git a/internal/workflow_testsuite_test.go b/internal/workflow_testsuite_test.go index 2f8af03bc..f2b7b9152 100644 --- a/internal/workflow_testsuite_test.go +++ b/internal/workflow_testsuite_test.go @@ -1234,6 +1234,43 @@ func TestDynamicWorkflows(t *testing.T) { require.Equal(t, "dynamic-activity - grape - cherry", result) } +func SleepHour(ctx Context) error { + // We need to specifically have a timer that's cancelled, so that the + // timer's underlying channel is closed when the workflow is cancelled + return Sleep(ctx, time.Hour) +} + +func SleepThenCancel(ctx Context) error { + cwf := ExecuteChildWorkflow( + ctx, + SleepHour, + ) + var res WorkflowExecution + if err := cwf.GetChildWorkflowExecution().Get(ctx, &res); err != nil { + return err + } + + err := RequestCancelExternalWorkflow(ctx, res.ID, res.RunID).Get(ctx, nil) + if err != nil { + return err + } + + // This Sleep yields control back to startMainLoop, allowing it to process the + // child's cancel callback. Without it, the parent completes before the cancel + // propagates. The cancel callback then calls workflowCancelHandler, + // which would previously cause a panic. + return Sleep(ctx, time.Second) +} + +func TestRequestCancelExternalWorkflowInSelector(t *testing.T) { + testSuite := &WorkflowTestSuite{} + env := testSuite.NewTestWorkflowEnvironment() + env.RegisterWorkflow(SleepHour) + env.ExecuteWorkflow(SleepThenCancel) + require.NoError(t, env.GetWorkflowError()) + env.IsWorkflowCompleted() +} + func checkActivityInfo(ctx context.Context, isWorkflowActivity bool) error { info := GetActivityInfo(ctx) if isWorkflowActivity {