sources: reproduce intermittent "unknown package" errors under concurrent execs (1/4) - #589
Open
jason-rl wants to merge 2 commits into
Open
sources: reproduce intermittent "unknown package" errors under concurrent execs (1/4)#589jason-rl wants to merge 2 commits into
jason-rl wants to merge 2 commits into
Conversation
Executing a Hermit-managed binary that hasn't been installed yet, several times within a few milliseconds of each other, can make some invocations fail with "unknown package" even though the package is perfectly valid. GitSource.Sync has no cross-process or cross-goroutine locking, so concurrent syncs of the same not-yet-cloned source race: each clones independently and then wipes and replaces the shared manifest tree, leaving a window where a concurrent reader sees ENOENT partway through. TestConcurrentSyncInProcess and TestConcurrentSyncAcrossProcesses reproduce this directly (the latter across genuine child processes, since util/flock is deliberately re-entrant per-PID and so cannot exercise cross-process contention from goroutines alone). Both fail against the current implementation; the fix follows in a subsequent change.
jason-rl
changed the title
TestConcurrentSyncInProcess and TestConcurrentSyncAcrossProcesses previously let goroutines/child processes begin racing as soon as each was spawned, so on a fast machine some finished before the last one even started, understating how often the race actually reproduces. Hold every goroutine/child at a barrier until all have signalled ready, then release them together, so all n consistently race through Sync concurrently. Also corrects TestConcurrentSyncInProcess's doc comment, which overclaimed that -race specifically exercises "the process-local mutex in sources/lock.go" -- that file doesn't exist yet at this point in the stack.
jason-rl
marked this pull request as ready for review
July 27, 2026 22:55
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
1 participant
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.
Summary
Running a Hermit-managed binary that hasn't been installed yet several times within a few milliseconds of each other can make some invocations fail with:
even though the package is perfectly valid.
GitSource.Sync(sources/git.go) has no cross-process or cross-goroutine locking, so concurrent syncs of the same not-yet-cloned manifest source race: every caller passes the same pre-sync check, clones independently, and each then wipes and replaces the shared manifest tree withos.RemoveAll+os.Rename-- an unlink storm that a concurrent reader can observe mid-way through asENOENT.This is 1 of 4 stacked PRs fixing this and three adjacent races found while investigating it -- see #593 for a reproduction of each. This PR adds only a regression test reproducing this race; it's expected to fail CI as-is, that's the point, it pins the bug down with a test before the fix (#590) lands. Please review it standalone; #590, #591, #592 build on top and are up for review too (each explains which commits are new to it). Both commits here are this PR's own (it's the base of the stack).
TestConcurrentSyncInProcess: N goroutines raceGitSource.Syncagainst one shared source dir; a background reader fails the test if it ever sees a manifest it already found disappear.TestConcurrentSyncAcrossProcesses: the same, but across genuine child processes (re-exec of the test binary) --util/flockis deliberately re-entrant per-PID, so goroutines alone can't exercise real cross-process lock contention.nhave been spawned and signalled ready, then all are released together), so the race is exercised on every run instead of being at the mercy of scheduling/launch-order luck.Test plan
go build ./...go test ./sources/... -run TestConcurrentSync -v-- both tests fail against currentmaster, reproducing the reported symptomThis PR -- the investigation, code, and tests -- was drafted with AI assistance (Claude Code).