Write event.json once per job instead of once per step - #4643
Open
MarshallOfSound wants to merge 1 commit into
Open
Write event.json once per job instead of once per step#4643MarshallOfSound wants to merge 1 commit into
MarshallOfSound wants to merge 1 commit into
Conversation
Every step start rewrote _github_workflow/event.json with identical content. With background/parallel steps that write races with sibling steps: a sharing violation on Windows, and a truncated read of GITHUB_EVENT_PATH on macOS/Linux. Write it once per job, only rewrite if it has gone missing, and make the write atomic via temp file + rename.
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
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.
Fixes #4641
ActionRunner.RunAsync()callsExecutionContext.WriteWebhookPayload()for every step —run:,uses:, job hooks, and each inner step of a composite — and that does a plainFile.WriteAllText(<temp>/_github_workflow/event.json, …). Harmless while steps were sequential; with background /parallel:steps it races:IOException: The process cannot access the file '…\event.json' because it is being used by another process(Background actions frequently fail on Windows due to locking event.json #4641).WriteAllTextis truncate-then-write, so a sibling step's process reading$GITHUB_EVENT_PATHin that window sees an empty/partial file. Anything using@actions/githubdies at import withSyntaxError: Unexpected end of JSON input at new Context (…)— details and a hosted-runner log in Background actions frequently fail on Windows due to locking event.json #4641 (comment).github.eventis delivered in the job message and the worker never modifies it (the onlySetGitHubContextwriters areaction*,event_path,workspace, the*_urls and file commands), so the content being written is identical for every step of the job. This changesWriteWebhookPayload()to:Globalsince background steps reach it from multiple threadsgithub.event_pathon every step's context, since background and composite steps get aShallowCopy()File.Move(overwrite: true)so a reader can never observe it half-writtenI kept the write lazy rather than moving it into
ExecutionContext.InitializeJobbecauseTempDirectoryManager.InitializeTempDirectoryruns after that inJobRunnerand can clean_temp.Side effect: every step start no longer does a
DictionaryContextData->JToken-> indented-string serialization plus a synchronous file write of the whole payload. Forpull_request-triggered jobs that's tens of KB per step, more noticeable in jobs with lots of small composite steps than anything else, but it was pure waste.Added L0 coverage for: write +
event_pathset, no rewrite on subsequent steps, rewrite after the file is removed, and concurrent callers with a reader polling the file (the reader observes 0-length reads onmain, none with this change).