Repository navigation
Conversation
… support Code review findings, fixed: * Linux: a live lock owner was reported as an orphan, so any waiter stole its write lock. Process.StartTime is derived per process from a wall-clock boot-time snapshot, so the value an owner records and the value another process computes for it differ by milliseconds and never compare equal. Record and compare /proc/<pid>/stat starttime (kernel ticks) instead. Values written by 3.0.0 (negative DateTime binaries) are treated as unknown and fall back to the PID-only check. * Orphan recovery only probed the owner at the start of a wait and at 75% of the timeout, so a wait with Timeout.InfiniteTimeSpan never recovered from an owner that died later. Probe every 250 ms instead. * Dispose() unmapped the header while another thread was still spinning in TryAcquireWriteLock/TryAcquireReadLock, which is a process-fatal AccessViolationException. Lock waiters now register with Dispose(), leave with ObjectDisposedException, and Dispose() waits for them before unmapping. * ReleaseWriteLock silently ignored a release from a non-owner thread, so `await` inside a lock scope left the lock held forever with no diagnostic. It now throws SynchronizationLockException, as do the StructuredMemory WriteLock/ReadLock guards when disposed on another thread (before touching any state, so the owning thread can still release correctly). * TypeLayoutFingerprint used Marshal.SizeOf/OffsetOf, which reject generic types, so ValueTuple/KeyValuePair<,> failed in every typed container. Fall back to a managed-layout descriptor for those types. Fingerprints of types that already worked are byte-for-byte unchanged (pinned by a test), so 3.0.0 and newer processes can still share regions. Tests: cross-process regression tests (live owner, infinite and finite wait recovery) with a new hold_write_lock worker role, Dispose-with-waiter, guard-on-another-thread, generic struct and pinned fingerprint tests. Two tests that asserted the old "silently ignored" behaviour now expect the exception. Co-Authored-By: Claude Sonnet 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
* OrphanLockTimeout now defaults to TimeSpan.Zero. The previous default of
30 s let a waiter take the write lock from a healthy process that held it
longer than that (a long transaction, a paused debugger), breaking mutual
exclusion. Recovery of a lock whose owner process has exited is unaffected
and stays on by default. DefaultOrphanLockTimeout keeps its value and is
documented as the suggested value when opting in.
* Read locks are not attributed to an owner, so a process that dies holding
one leaves the shared reader count above zero forever; on Linux the
/dev/shm file even outlives every user. This cannot be detected
automatically, so give operators explicit tools:
- MemoryRegion.ForceResetLocks() / StructuredMemory<T>.ForceResetLocks()
clear the write lock and the reader count.
- LockOwnerInfo.ReaderCount exposes the count for diagnosis.
- MemoryRegion.Remove(name, options) deletes the backing storage (Linux
/dev/shm file or an explicit FilePath) so a region left unusable by a
crashed creator, or one with an unwanted capacity or element type, can be
recreated. The initialization timeout message now points to it.
README documents both the new default and the recovery procedure.
Tests: cross-process dead-reader scenario (new hold_read_lock worker role),
ForceResetLocks for MemoryRegion and StructuredMemory, Remove for Linux
regions, file-backed regions and a region stuck in the initializing state.
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
zacala1
changed the title
Disposing a region while another thread is inside Read/Write or a typed queue unmaps memory that thread is still using, which terminates the process with an AccessViolationException. Measured with three threads busy polling a ConcurrentQueue on a 4-core machine, Dispose killed the process in 12 of 12 runs of 400 races each. Tracking in-flight calls would close the race but is not affordable on the lock-free paths: two interlocked operations per call made a single-thread queue round trip about 9x slower (3.3 ns -> 31 ns), two threads exchanging items about 17x slower (about 70 M -> 4 M msgs/s) and MemoryRegion.Read about 2x slower. Striping the counter per CPU did not help. Instead, MemoryRegion.Dispose now keeps the mapping for MemoryRegion.DisposeGracePeriod (default 10 ms, process-wide, Zero disables) after the instance is marked disposed. New calls already fail with ObjectDisposedException, so the pause lets calls that were past their check finish. Measured: 12 of 12 runs of 400 races survive with the default, and 8 oversubscribed pollers on 4 cores still failed in 1 of 8 runs (0 of 8 at 25 ms). It is a mitigation, not a guarantee, and says so in the XML docs and the README: the only complete protection is to stop and join every thread that uses an instance before disposing it. Tests: the grace period setting, that Dispose honours it, and a busy-poll Dispose race that kills the process without the grace period. Co-Authored-By: Claude Sonnet 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
zacala1
changed the title
* SharedArray did not dispose its region when the header check failed (another element type or length), so the mapping and, on Linux, its file descriptor stayed open until the finalizer ran. The other containers already clean up; do the same here. * SharedArray and StructuredMemory expose no statistics, yet their region updated its read/write counters with interlocked operations on every access. Turn the counters off for these internal regions: no observable change, SharedArray<long> get 24 -> 18 ns and set 15.6 -> 5.9 ns. * Every automatically locked StructuredMemory call (values wider than 8 bytes, strings, blobs, arrays) allocated 104 bytes: a new delegate for the lock guard (method group conversion) plus a Stopwatch instance inside the lock wait. Cache the delegates per instance and time the lock waits with Stopwatch timestamps. Measured 104 -> 0 bytes per call and Write<Guid> 311 -> 190 ns. Tests: failed open leaves no extra file descriptor (Linux), and automatically locked access allocates nothing per call. Co-Authored-By: Claude Sonnet 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
ConcurrentMessageQueue, SingleProducerByteStream, SharedArray and StructuredMemory had finalizers that only set a flag or disposed a MemoryHandle, which wraps an unmanaged pointer and owns nothing. They made every instance finalizable, which costs an extra GC promotion, without freeing anything: when Dispose is never called, the MemoryRegion's own finalizer already unmaps the memory. Also drop GC.SuppressFinalize from SingleProducerQueue and ConcurrentQueue, which never had a finalizer. Co-Authored-By: Claude Sonnet 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
SingleProducerQueue, ConcurrentQueue and ConcurrentMessageQueue each carried a private bit-twiddling copy of RoundUpToPowerOf2. Use BitOperations through one internal helper instead. Behaviour is unchanged except that an out-of-range capacity now reports the parameter name "capacity" and the offending value instead of "value", and the capacity-mismatch check computes the rounded value once. Co-Authored-By: Claude Sonnet 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
Both types wrote their header fields and then the magic with two plain region writes, and read the whole header in one copy while polling for the magic. MemoryRegion.Write/Read do no fencing, and a single copy gives no ordering between the magic and the bytes after it, so on a weakly ordered CPU (ARM) an opener could see the magic before the fields and fail with a spurious "different format" error. The queues already publish their magic last with a release write. Insert a full fence before the magic is written, and on the reader poll the 4-byte magic alone, then fence and read the fields. This is a no-op on x86, and the ordering cannot be exercised on the x86 machine used here; the existing open/create tests cover the unchanged behaviour. Co-Authored-By: Claude Sonnet 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
* MPMC_BurstTraffic_ShouldHandleSpikes: the consumer gave up after 10,000
empty polls, which can be used up in microseconds when it starts before
the producer. The producer then spun forever on a full queue and the test
hung (about 45% of isolated runs). The consumer now waits for the full
burst, and both sides stop with an error after 30 s instead of hanging.
* MPMC_HighContention_ShouldMaintainIntegrity: same give-up heuristic, plus
thread-pool starvation. Four spinning producers on Task.Run occupy every
pool worker of a small machine, the consumers cannot start for seconds,
and the producers give up on the full queue ("stuck at message 256",
which is the slot count). It failed 25 of 25 isolated runs in a fresh
process and only passed inside the full suite because earlier tests had
already grown the pool. Producers and consumers now run on dedicated
threads (TaskCreationOptions.LongRunning) and consumers wait for the full
count with a deadline.
Both pass 15 of 15 isolated runs afterwards. No library change.
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
zacala1
changed the title
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
2 participants
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.
Superseded by #3: same changes, moved to the branch
fix/lock-safety-and-crash-recoverywith commits authored under the repository owner's identity. Closing this one.