Fix data race in UpdateWithStart: read ExecutionState.Status before releasing workflow lock - #11602
Merged
yiminc merged 3 commits intoAug 19, 2026
Conversation
…eleasing workflow lock Capture ExecutionState.Status before calling GetReleaseFn()(nil) to avoid a data race with concurrent goroutines that may acquire the lock and modify ExecutionState after it is released. Add a regression test that confirms the race under -race.
Lakshaymiddha
force-pushed
the
fix/multioperation-status-data-race
branch
from
August 17, 2026 19:22
7a5f71e to
59fd185
Compare
yiminc
approved these changes
Aug 19, 2026
davidporter-id-au
pushed a commit
to davidporter-id-au/temporal
that referenced
this pull request
Aug 24, 2026
…eleasing workflow lock (temporalio#11602) ## What changed? Captured `ExecutionState.Status` before calling `GetReleaseFn()(nil)` in `updateWithStart.Invoke` to avoid a data race with concurrent goroutines that may acquire the lock and modify `ExecutionState` after it is released. Added a regression test that confirms the race under `-race`. Fixes temporalio#11600 ## Why? `Invoke` in `service/history/api/multioperation/api.go` 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. This matches the pattern noted in the `Updater` struct itself: > WARNING: any references to mutable state data *have to* be copied to avoid data races when used outside the workflow lease. ## How did you test it? - [x] added new unit test(s) The test spawns a concurrent writer that modifies `ExecutionState.Status` after the lock is released. **Before fix:** ``` go test -race -tags test_dep -count=1 \ -run TestUpdateWithStartSuite/TestInvoke_CompletedUpdate_StatusCapturedBeforeRelease \ ./service/history/api/multioperation/ WARNING: DATA RACE Read at ... api.go:201 --- FAIL ``` **After fix:** ``` ok go.temporal.io/server/service/history/api/multioperation ``` ## Potential risks Minimal - single line moved before the release call. No API or persistence behavior change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed?
Captured
ExecutionState.Statusbefore callingGetReleaseFn()(nil)inupdateWithStart.Invoketo avoid a data race with concurrent goroutines that may acquire the lock and modifyExecutionStateafter it is released.Added a regression test that confirms the race under
-race.Fixes #11600
Why?
Invokeinservice/history/api/multioperation/api.goreleases the workflow lock at line 196 viaworkflowLease.GetReleaseFn()(nil), then readsworkflowLease.GetMutableState().GetExecutionState().Statusat line 201 — after the lock is released. Any concurrent goroutine waiting onLock()for the same workflow (e.g. a signal, terminate, or another update) can acquire the lock and modifyExecutionStatebetween lines 196 and 201, creating a data race.This matches the pattern noted in the
Updaterstruct itself:How did you test it?
The test spawns a concurrent writer that modifies
ExecutionState.Statusafter the lock is released.Before fix:
After fix:
Potential risks
Minimal - single line moved before the release call. No API or persistence behavior change.