Skip to content

Data race in UpdateWithStart: ExecutionState.Status read after workflow lock release #11600

Description

@Lakshaymiddha

Expected Behavior

Invoke in service/history/api/multioperation/api.go should read all mutable state fields while the workflow lock is held, matching the lock-before-read pattern used by MergeSlices, ClearSlices, and other methods across the codebase.

Actual Behavior

When Invoke finds a completed update outcome, it releases the workflow lock at line 196 via workflowLease.GetReleaseFn()(nil), then reads workflowLease.GetMutableState().GetExecutionState().Status at line 201 — after the lock is released. Any concurrent goroutine
waiting on Lock() for the same workflow (e.g. a signal, terminate, or another update) can acquire the lock and modify ExecutionState between lines 196 and 201, creating a data race.

// service/history/api/multioperation/api.go:194-204
if outcome, err := workflowLease.GetMutableState().GetUpdateOutcome(ctx, updateID); err == nil {
    workflowKey := workflowLease.GetContext().GetWorkflowKey()
    workflowLease.GetReleaseFn()(nil)               // line 196: lock released                                                                                                                                                                                                
    return makeResponse(
        &historyservice.StartWorkflowExecutionResponse{                                                                                                                                                                                                                       
            RunId:   workflowKey.RunID,
            Started: false,
            Status:  workflowLease.GetMutableState().GetExecutionState().Status, // line 201: read after release
        },                                                                                                                                                                                                                                                                    
        uws.updater.CreateResponse(workflowKey, outcome, enumspb.UPDATE_WORKFLOW_EXECUTION_LIFECYCLE_STAGE_COMPLETED),
    ), nil                                                                                                                                                                                                                                                                    
}                                      

The fix is to capture Status before releasing the lock:

  status := workflowLease.GetMutableState().GetExecutionState().Status
  workflowLease.GetReleaseFn()(nil)                                                                                                                                                                                                                                             
  // ... use captured `status` in the response

This matches the pattern noted in the Updater struct itself:

Steps to Reproduce the Problem

  1. Add a test to service/history/api/multioperation/api_test.go that mocks a workflow lease where a completed update outcome is returned, and spawns a concurrent writer that modifies ExecutionState.Status after the lock is released.
  2. Run: go test -race -tags test_dep -count=1 -run 'TestUpdateWithStartSuite/TestInvoke_CompletedUpdate_StatusCapturedBeforeRelease' ./service/history/api/multioperation/
  3. Observe the race report: Invoke reads ExecutionState.Status at api.go:201 racing with the concurrent writer.

Specifications

  • Version: current main (updateWithStart.Invoke, service/history/api/multioperation/api.go)
  • Platform: darwin/arm64

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions