Found during PR #1102's review (CodeRabbit).
with_hub_read_lock holds hub_read_lock_acquire's shared lock (flock -s)
for its whole span, including ensure_hub_root's own freshness check, which
runs git fetch against the tentatively-reused hub checkout (host-setup/menu.sh,
around the git -C "$HUB_ROOT" fetch --quiet origin "$DEFAULT_REF" call). Since
a shared lock permits multiple concurrent holders, two sessions can run this
git fetch against the same checkout at the same time, updating
FETCH_HEAD and the remote-tracking ref concurrently.
Git's own fetch machinery has some internal locking (packed-refs, lock files
per ref), so this is not necessarily a data-corrupting race, but it is a
real, unreviewed concurrent-write hazard this reader/writer lock design was
not built to allow, and CodeRabbit rates it Major.
Suggested directions (not yet designed): move this specific fetch under
the exclusive lock (defeats some of the reader-concurrency the shared-lock
design exists for, since freshness verification happens on every read task,
not just an explicit fetch_hub call), or add a narrower, fetch-specific
lock distinct from the read/use lock this PR's own chain already introduced.
Deferred out of the PR #1091/#1098/#1100/#1102 chain as a heavier design
lift than that chain's own scope (closing concrete rule-6 git-command
bypasses and the reader/writer lock's own precision), tracked here for a
follow-up pass.
Found during PR #1102's review (CodeRabbit).
with_hub_read_lockholdshub_read_lock_acquire's shared lock (flock -s)for its whole span, including
ensure_hub_root's own freshness check, whichruns
git fetchagainst the tentatively-reused hub checkout (host-setup/menu.sh,around the
git -C "$HUB_ROOT" fetch --quiet origin "$DEFAULT_REF"call). Sincea shared lock permits multiple concurrent holders, two sessions can run this
git fetchagainst the same checkout at the same time, updatingFETCH_HEADand the remote-tracking ref concurrently.Git's own fetch machinery has some internal locking (packed-refs, lock files
per ref), so this is not necessarily a data-corrupting race, but it is a
real, unreviewed concurrent-write hazard this reader/writer lock design was
not built to allow, and CodeRabbit rates it Major.
Suggested directions (not yet designed): move this specific fetch under
the exclusive lock (defeats some of the reader-concurrency the shared-lock
design exists for, since freshness verification happens on every read task,
not just an explicit
fetch_hubcall), or add a narrower, fetch-specificlock distinct from the read/use lock this PR's own chain already introduced.
Deferred out of the PR #1091/#1098/#1100/#1102 chain as a heavier design
lift than that chain's own scope (closing concrete rule-6 git-command
bypasses and the reader/writer lock's own precision), tracked here for a
follow-up pass.