Open
Conversation
Batch completion could race when multiple job completions reused the
same `JobStatistics` pointer. Short excerpt from the race detector
failure in a recent CI run:
WARNING: DATA RACE
Write at ... BatchCompleter.handleBatch.func4()
.../internal/jobcompleter/job_completer.go:482
Previous read ... jobStatisticsFromInternal()
.../event.go:75
`BatchCompleter` queued the caller's stats pointer directly and later
mutated `CompleteDuration` before publishing updates. Batch job error
paths can reuse one stats object across multiple completions, so a later
batch could mutate stats that were already visible to subscribers.
Take a snapshot of `JobStatistics` when queuing the completion so each
published update owns an immutable copy. Add regression coverage that
queues two completions with a shared stats pointer and verifies that
each published update has a distinct snapshot.
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
2 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.
Batch completion could race when multiple job completions reused the
same
JobStatisticspointer. Short excerpt from the race detectorfailure in a recent CI run:
BatchCompleterqueued the caller's stats pointer directly and latermutated
CompleteDurationbefore publishing updates. Batch job errorpaths can reuse one stats object across multiple completions, so a later
batch could mutate stats that were already visible to subscribers.
Take a snapshot of
JobStatisticswhen queuing the completion so eachpublished update owns an immutable copy. Add regression coverage that
queues two completions with a shared stats pointer and verifies that
each published update has a distinct snapshot.