Races on the "hermit exec" hot path: intermittent "unknown package" (and two related failures)

[jason-rl]

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:

fatal:hermit: <name>: unknown package

even though the package is perfectly valid.

Investigating this surfaced four related races on the same "hermit exec" hot path. Races A and B are the confirmed, direct cause of the unknown package message above -- both go through manifest/loader.go's ErrUnknownPackage path. Races C and D are separate, adjacent races found during the investigation: they don't produce unknown package -- they surface (when they surface at all) as a different fatal error, "<pkg>: failed to execute \"<bin>\": no such file or directory" (env.go's syscall.Exec wrapper) -- but they're real correctness bugs on the same "thing you just ran may not actually be ready yet" hot path, so they're included and fixed here too.

All four are reproduced below directly against unpatched master, with captured output, and fixed as four independently-reviewable, stacked PRs.

---

Race A -- concurrent GitSource.Sync destroys the manifest tree a reader is using

Produces the reported unknown package error.

sources/git.go's Sync/syncGit has no cross-process or cross-goroutine locking. N processes/goroutines syncing the same not-yet-cloned source all clone independently, and each then wipes and replaces the shared manifest tree with os.RemoveAll(finalDest) + os.Rename(dest, finalDest). A reader using os.DirFS(finalDest) concurrently can observe ENOENT mid-replace, which manifest/loader.go's load() treats as "manifest not found" -- indistinguishable, at that point, from a package that never existed -- and reports as ErrUnknownPackage, i.e. exactly the error text above.

Steps to reproduce: 8 processes (or goroutines) call GitSource.Sync concurrently against the same not-yet-cloned source directory, while a reader repeatedly reads a manifest file from that directory.

Expected: a manifest a reader has already successfully found should never subsequently vanish.

Actual (captured against master):

=== RUN   TestConcurrentSyncInProcess
    git_concurrency_test.go:171: Did not expect an error but got:
        manifest disappeared after first appearing: .../2ea20b82.../pkg.hcl
--- FAIL: TestConcurrentSyncInProcess (0.06s)
=== RUN   TestConcurrentSyncAcrossProcesses
    git_concurrency_test.go:224: Did not expect an error but got:
        manifest disappeared after first appearing: .../7c2cdea7.../pkg.hcl
--- FAIL: TestConcurrentSyncAcrossProcesses (1.14s)

Reproduced both with goroutines sharing a process and with genuine separate child processes.

Fixed by #589 (regression test) + #590 (fix).

---

Race B -- a transiently-missing source directory is misreported as a permanently unknown package

Also produces the reported unknown package error -- this is the loader-side companion to Race A, widening coverage to mixed-Hermit-version fleets and any residual window in the fix for Race A (e.g. an older Hermit binary sharing a state dir, which doesn't take the lock added for Race A).

manifest/loader.go's load() maps any os.ErrNotExist from a source -- whether just the single requested manifest file is missing, or the entire backing directory is transiently absent -- to "not found," which get() turns into ErrUnknownPackage. A package that genuinely exists in a healthy source is indistinguishable from one that never existed, if a resync (by any Hermit version) happens to catch it mid-swap.

Steps to reproduce: populate a real package manifest via a git source, confirm Loader resolves it, then concurrently repeat master's actual swap sequence (os.RemoveAll + recreate) against tight-loop Loader.get calls for that same, real, already-resolving package.

Expected: a package that exists in a healthy source should never be reported as unknown merely because another process is mid-resync.

Actual (captured against master):

observed foo (a real, existing package) reported as: foo: unknown package

Fixed by #591.

---

Race C -- internal/dao.UpdatePackage's non-atomic write yields a torn etag read

Does NOT produce "unknown package". If it surfaces at all, it's as "<pkg>: failed to execute \"<bin>\": no such file or directory" (env.go:912-913), because a torn etag read can make UpgradeChannel wrongly evictPackage (rm -rf) a package tree that another process is concurrently trying to exec. It can also be entirely silent (an unnecessary re-download/re-check) if no exec races the eviction.

UpdatePackage writes a package's cached etag with a plain os.WriteFile, which truncates the existing file before writing new content. A concurrent GetPackage can observe the file mid-write.

Steps to reproduce: one goroutine calls UpdatePackage in a loop with real-sized etag content, another concurrently calls GetPackage in a loop, asserting the returned etag is always a value that was actually, fully written.

Expected: a concurrent read of a package's cached etag should always see a complete, valid value (old or new), never a torn one.

Actual (captured against master): 30/30 runs (-count=30 -race) failed with:

observed torn/unexpected etag: ""

Fixed by #592.

---

Race D -- state.linkBinaries's destructive rebuild races areBinariesLinked's unlocked pre-check

Does NOT produce "unknown package". Same failure mode as Race C if it surfaces: "<pkg>: failed to execute \"<bin>\": no such file or directory" from env.go:912-913, since CacheAndUnpack may go on to exec a binary through a directory it only just observed (racily) as "already linked".

linkBinaries builds its symlink directory with os.RemoveAll + recreate. CacheAndUnpack's unlocked pre-lock fast path (areBinariesLinked) can observe "already linked" and then find the directory removed out from under it moments later, because another goroutine/process's linkBinaries call is mid-rebuild.

Steps to reproduce: run linkBinaries in a tight loop concurrently with a loop that calls areBinariesLinked() immediately followed by re-statting the link.

Expected: once the pre-lock check reports binaries as linked, that should remain true long enough for the caller to actually use them.

Actual (captured against master): 875 instances, in a 2-second window, of areBinariesLinked() == true immediately followed by a failed stat.

Fixed by #592.

---

Fixes

Race	Produces reported unknown package?	PR
A	Yes	#589 (repro), #590 (fix)
B	Yes	#591
C	No -- different error / silent	#592
D	No -- different error	#592

All four PRs are stacked (each explains, in its description, which single commit is new relative to the previous one) and are up for independent review.

---

This issue -- including the investigation and reproductions above -- was drafted with AI assistance (Claude Code).