fix(storage): refcount the process lock for same-process stores - #58
Merged
Conversation
flock(2)/LockFileEx is per-process, so the merged PR #47 lock rejected a second NewStore on the same DB within one process. Hosts that construct the store from multiple callsites (hawk opens a YaadBridge per callsite) would then silently lose memory access. - Keep the OS lock exclusive across processes (flock / LockFileEx). - Within a process, share it per lock path via a refcount registry: Acquire bumps, Release unrefs; the file is unlocked and removed only on the last release. - Release is idempotent; in-memory no-op locks are unchanged. - Tests: same-process sharing, refcount release ordering, and a cross-process sentinel test asserting another process is still blocked.
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.
The process lock merged in #47 used
flock(2)/LockFileExdirectly inNewStore/Close. These OS locks are per-process, so a secondNewStoreon the same DB within one process was rejected (verified: probe returneddatabase locked by another yaad process: resource temporarily unavailable).Hosts that construct the store from multiple callsites (hawk creates a
YaadBridgeper bridge) would then silently fail and lose memory access.Fix:
AcquireProcessLockbumps,Releaseunrefs; the file is unlocked + removed only on the last release.Releaseis idempotent;sync.Onceguards double release; in-memory no-op locks unchanged.Tests: same-process sharing (4 concurrent stores), refcount release ordering, idempotency, and a cross-process sentinel test proving another process is still blocked. Updated
TestProcessLockto the refcount semantics. Fullgo test ./...green, race-clean, gofumpt/lint clean, both platforms build.