Skip to content

Fix Linux lock-owner detection and lock-wait robustness; opt-in lock takeover; crash-recovery tools; narrow Dispose races - #3

Open
zacala1 wants to merge 29 commits into
mainfrom
fix/lock-safety-and-crash-recovery
Open

zacala1 wants to merge 29 commits into
mainfrom
fix/lock-safety-and-crash-recovery

Conversation

@zacala1

@zacala1 zacala1 commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fixes from three code reviews of MemoryRegion and the typed containers. Each item was reproduced before it was fixed, and the new tests fail on the previous code. Small commits, each reviewable on its own. CHANGELOG.md has the complete list.

First round (lock handling and the first review): items 1-10 below, plus CI matrix, docs and CHANGELOG.md.

Second round (five independent reviewers over the whole code base): R1-R7.

Third round (another full review): R8-R13.

# Problem Fix
1 Linux: a live write-lock owner was reported as an orphan, so any waiter stole its lock. Process.StartTime differs between observers by milliseconds and never compares equal. Record and compare /proc/<pid>/stat starttime (kernel ticks) on Linux. Values written by 3.0.0 are treated as "unknown" and fall back to the PID-only check. Windows unchanged.
2 await inside a lock scope leaked the write lock forever. ReleaseWriteLock silently ignored a release from a non-owner thread. ReleaseWriteLock throws SynchronizationLockException; the StructuredMemory guards throw the same when disposed on another thread, before touching any state.
3 Dispose() while another thread waited for a lock crashed the process (uncatchable AccessViolationException). Lock waiters register with Dispose(), leave with ObjectDisposedException, and Dispose() waits for them before unmapping.
4 Default OrphanLockTimeout = 30 s let a waiter take the lock from a healthy owner. Defaults to TimeSpan.Zero (opt-in). Recovery of a lock whose owner process has exited is still on by default.
5 Orphan probe only at the start and at 75% of the timeout; Timeout.InfiniteTimeSpan never recovered from an owner that died later. Probe every 250 ms.
6 A process that dies holding a read lock leaves the reader count above zero forever; no way to remove a region. ForceResetLocks(), LockOwnerInfo.ReaderCount, MemoryRegion.Remove(name, options), README "Recovering after a crash".
7 Dispose() racing lock-free calls crashed the process (12 of 12 runs of 400 races). Mitigation only: Dispose() keeps the mapping for DisposeGracePeriod (default 10 ms). See below.
8 Generic unmanaged structs (ValueTuple, KeyValuePair<,>) threw in every typed container. Managed-layout descriptor for those types only; fingerprints of types that already worked are unchanged (pinned by a test).
9 SharedArray leaked its region when the header check failed; avoidable per-call cost. Dispose on failure, internal statistics off, cached guard delegates, timestamps instead of Stopwatch.
10 SharedArray<T> elements wider than 8 bytes could be read torn by another process. Wide elements are accessed under the shared region lock; explicit reentrant AcquireReadLock/AcquireWriteLock (ref struct guards).
R1 2 byte elements could be read half written (a two byte Span.CopyTo is a one byte plus a two byte store). StructuredMemory also tore 3/5/6/7 byte values and small arrays. AtomicAccess: one Volatile load/store of exactly 1/2/4/8 bytes; StructuredMemory locks every other size and every array.
R2 A write lock whose owner was killed before recording its pid stayed held forever; readers never recovered a dead writer. A waiter clears a lock with no owner after two seconds; readers share the writer's probe.
R3 MPMC queues with capacity 1 overwrote an item and then delivered nothing again. Minimum two slots; a stored capacity below 2 is rejected.
R4 Linux: a region larger than the free space of /dev/shm ended in SIGBUS at the first write that did not fit (Docker default 64 MB). IOException up front with a --shm-size hint; FilePath mode checked too.
R5 Containers sharing /dev/shm took each other's locks (waiter looked the pid up in its own PID namespace). The owner records the inode of /proc/self/ns/pid (header offset 88, formerly reserved); a waiter in another namespace skips the pid check.
R6 StructuredMemory.AcquireWriteLock under a read guard blocked everyone until timeout; SingleProducerByteStream.Available/Used had no disposed check; MPMC timeout overloads counted a failure on every poll. Throw at once; add ThrowIfDisposed; count once when giving up.
R7 Test and CI harness: thread-pool starvation from Task.Run spin loops, silent continue-on-error, orphaned helper processes. DedicatedThread, report step with warning annotation and summary, bounded waits, hold_* children end when the parent dies, CI fails if the worker DLL is missing.
R8 Region creation races (Linux). Two CreateOrOpen calls for a new name both became creator and the loser deleted the winner's file; OpenExisting racing the creator reported "invalid header" (about 9% of races). FileMode.CreateNew decides the creator and only it removes the file on failure; openers wait up to 2 s for a region that is still being created.
R9 FilePath mode grew an existing file of another size before validating it (permanent, irreversible); GetMemory did not keep the region alive, so the finalizer could unmap memory still in use; zombie lock owners counted as alive. Reject without modifying, open with sharing; the returned Memory<byte> roots the region; /proc state Z is "gone".
R10 Lock guards of StructuredMemory and SharedArray: disposing a copy twice decremented the thread's depth again; releasing a write guard before a read guard inside it removed the protection; disposing the container with an open guard left the lock held until process exit. Guards remember the depth they were taken at and refuse copies and out-of-order release (SynchronizationLockException, nothing changes, guard stays valid); Dispose releases the calling thread's lock.
R11 StructuredMemory.OpenExisting checked the size before the version, so Forward/Full could not open a region of another size and Strict reported size instead of version. Version first; same version needs the exact size, different versions need regionSize >= schema size. README "Schema versions".
R12 SharedArray.Fill/Clear threw TypeLoadException for elements of 64 KiB or more and staged up to 128 MiB for 32 KiB elements. A by-value 100 KiB struct also overflowed the 1 MiB Windows stack in CI. Large elements are written one by one; batches limited to 64 KiB; in parameters and NoInlining helpers (verified with ulimit -s 1024).
R13 Docs: a crash mid-claim in an MPMC queue stalls the slot (undocumented); mixing 3.0.0 with later versions on Linux makes 3.0.0 steal locks. Documented in XML docs, README and CHANGELOG (recovery = MemoryRegion.Remove).

Why item 7 is a mitigation and not a fix

Tracking in-flight calls would close the race, but those paths are lock-free and deliberately do no bookkeeping. Measured cost of two interlocked operations per call: SingleProducerQueue enqueue+dequeue 3.3 ns -> 31 ns (about 9x), two threads exchanging items about 70 M -> 4 M msgs/s (about 17x). So Dispose pauses instead of counting. The only complete protection is to stop and join every thread that uses an instance before disposing it; the README and XML docs say so.

Behaviour changes to be aware of

  • ReleaseWriteLock by a thread that does not own the lock throws instead of being ignored.
  • OrphanLockTimeout defaults to disabled.
  • Dispose() takes at least DisposeGracePeriod (10 ms) longer, as does every container that owns a region.
  • SharedArray<T> with element sizes other than 1/2/4/8 locks per element access.
  • StructuredMemory<T>: only 1, 2, 4 and 8 byte scalars are lock-free; every array and every other size takes the shared lock.
  • MPMC queues: a requested capacity of 1 becomes 2; an existing region that stored 1 is rejected (MemoryRegion.Remove).
  • Linux: CreateOrOpen throws IOException when /dev/shm cannot hold the region.
  • StructuredMemory.OpenExisting checks the version before the size (R11); a region with a different size is no longer rejected for that reason alone when the versions differ.
  • FilePath mode rejects an existing file of another size instead of growing it.
  • A waiting reader recovers a write lock whose owner died; a lock with no owner is cleared by a waiter after two seconds.
  • Guards refuse a second dispose of a copy and out-of-order release (SynchronizationLockException); StructuredMemory.AcquireWriteLock() under a read guard throws InvalidOperationException.
  • New public API: MemoryRegion.Remove, MemoryRegion.ForceResetLocks, MemoryRegion.DisposeGracePeriod, StructuredMemory<T>.ForceResetLocks, LockOwnerInfo.ReaderCount, SharedArray<T>.AcquireReadLock/AcquireWriteLock/ForceResetLocks and the nested guards. IMemoryRegion is unchanged. The format version is unchanged; older processes ignore the field at header offset 88.

Verification

CI (.github/workflows/ci.yml, ubuntu-latest and windows-latest). Head commit c36dede, run 37887221001, both jobs green:

blocking step (everything except TimingSensitive/LongRunning) informational TimingSensitive step
ubuntu-latest 560 passed, 0 failed, 0 skipped 1 passed, 1 failed
windows-latest 547 passed, 0 failed, 13 skipped (Linux-only tests) 1 passed, 1 failed

The failing one is Fairness_Mpmc_ProducersGetReasonableShare (producer share ratio 3.5 on Linux and 31.6 on Windows against a limit of 3). It asserts a property the Vyukov queue does not promise (eight spinning producers on a few cores let the running one win the next slot again and again). It stays informational and shows up as a warning annotation plus a run summary. The test that races Dispose against busy-polling threads runs in a child process and passes on both.

Locally (Linux, 4 cores, NUnit shim because NuGet is blocked there): 560 passed, 2 failed, 4 explicit/skipped; the two are shim-only (GetFieldNames_ShouldReturnAllFields, ExportedApi_UsesOnlyVersion3NamesAndNamespace) and pass in CI. With ulimit -s 1024 (Windows stack size) the suite shows no stack overflow. Every new test was run against the previous code and fails there.

Not changed here (decisions for the author)

  • StructuredMemory lock guards are plain structs, so an await inside a guarded scope compiles and then fails loudly at dispose (the lock stays with the thread that took it). SharedArray guards are ref structs. Switching StructuredMemory is a public API break.
  • The package version is still 3.0.0 while README, MIGRATION and CHANGELOG describe unreleased behaviour; a strict reading of semver suggests 4.0.0. Until then, 3.0.0 and newer processes must not share a region on Linux.
  • Fairness_Mpmc_ProducersGetReasonableShare: relax the limit, delete it, or keep it as an informational warning.
  • Orphaned read locks still cannot be recovered automatically (needs a header format change). Dispose vs the lock-free members is mitigated, not eliminated. ConcurrentQueue<T> collides in name with System.Collections.Concurrent.ConcurrentQueue<T>.
  • Remaining minor review findings (stale DataLength re-check in the queues, Sleep(1) latency, WriteUtf8String("") leaving old bytes, 0644 permissions, no Global\ names, ...) are not part of this PR.
  • Windows-specific assumptions are covered only by what the Windows CI run exercises: no cross-session or cross-account owner scenario.

vic-py added 8 commits October 3, 2026 17:57
… 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.
* 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.
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.
* 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.
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.
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.
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.
* 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.
@zacala1 zacala1 mentioned this pull request Oct 5, 2026
vic-py added 21 commits October 5, 2026 14:19
Elements wider than 8 bytes, or whose size is not a power of two, could be
read torn while another process wrote them: about 1% of reads of a 64-byte
element in a two-thread stress test (233,928 of 23.7 M). Nothing exposed the
shared region lock, so users had no way to prevent it. StructuredMemory
already locks such values; SharedArray did not.

* Elements of 1, 2, 4 or 8 bytes are copied with one aligned move and keep
  the lock-free path (a per-T constant, so the JIT drops the branch).
  Every other size is read and written under the shared region lock by the
  indexer, CopyTo, CopyFrom and Fill (a Fill takes it once for the range).
  The torn-read reproduction now shows 0 torn reads.

* AcquireReadLock/AcquireWriteLock([timeout]) for any element type, to read
  or change several elements as one unit. They are reentrant for the calling
  thread, so the indexer inside a lock does not take it again, and writing
  while holding only a read lock throws InvalidOperationException, as in
  StructuredMemory. The guards are ref structs: the lock belongs to one
  thread, and the compiler now rejects holding a guard in an async method
  (CS9104), which is the misuse that leaked write locks before.

* ForceResetLocks(), like MemoryRegion and StructuredMemory, to clear the
  lock state a crashed process left behind.

* The indexer no longer uses stackalloc (the JIT cannot inline methods that
  do), reading through a span over a local instead.

Behaviour change: arrays of wide or odd-sized elements now take a lock per
element access, about ten times fewer reads per second in the stress test,
in exchange for correct values. Lock-free element types are unaffected
(SharedArray<long> get/set are within measurement noise of before).
Processes running a version without this locking do not take part in it.

Tests cover torn reads, atomic types bypassing the lock, exclusion between
instances, reentrancy, read-to-write upgrade rejection, consistent snapshots
of several elements, double dispose of guards, ForceResetLocks, and round
trips of 64-byte and 3-byte elements through every access path.
Build and test on ubuntu-latest and windows-latest for pushes to main and
for pull requests. The gating test step skips tests tagged TimingSensitive
(a scheduling-fairness assertion) and LongRunning (the [Explicit] soak
tests); the timing-sensitive ones run in a separate informational step so a
slow shared runner does not fail the build. TRX results are uploaded for
every run.
Document the thread rule for the write lock, the opt-in OrphanLockTimeout,
read-lock recovery and the Dispose contract on IMemoryRegion, MemoryRegion,
StructuredMemory and every container's Dispose. Correct the header comment on
the recorded start time (platform specific encoding) and drop comments that
described how the code used to behave. Comments only, no code changes.
Describe the CI matrix and the TimingSensitive/LongRunning test categories,
point MIGRATION at MemoryRegion.Remove, and list the behavior changes,
fixes and additions since 3.0.0 in a new CHANGELOG.
Opt8_OptimisticReader_64Readers_4Writers_AllProgress started 68 loops that
spin until a token is cancelled with Task.Run. With few cores they occupy
every thread-pool worker, the pool adds a thread only every ~500 ms, and the
writers queued behind the readers start late or the cancellation timer
callback waits behind them. The test then fails with a starved writer or runs
into its 30 s timeout (seen on a GitHub Linux runner). It fails the same way
on the code before the lock changes.

LongRunning gives every loop its own thread. Passes 6 of 6 runs each pinned
to 1, 2 and 4 CPUs; before, it failed 6 of 6 runs on 2 CPUs.
Without a condition it was skipped when the blocking test step failed, so a
failing run showed nothing about the timing-sensitive tests. It cannot fail
the job (continue-on-error), so running it unconditionally is safe.
SharedArray and StructuredMemory copied small values through Span.CopyTo.
For a length of two bytes that is a one byte store followed by a two byte
store, so a reader in another process saw 0x0000 or 0xFFFF between 0x00FF
and 0xFF00 (about 1.5% of reads in a two thread test; 3 and 6 byte values
and small arrays in StructuredMemory were torn in the same way, because
only values wider than 8 bytes were locked).

AtomicAccess performs one Volatile load or store of exactly the element
width. SharedArray uses it for single elements (indexer, one-element
CopyTo/CopyFrom/Fill). StructuredMemory uses it for 1, 2, 4 and 8 byte
scalars and locks every other size and every array.

The new tests fail on the previous code (83,265 of 5.3 M reads torn for
ushort, 72,633 of 2.6 M for the StructuredMemory fields) and pass now.
…writers

The write lock is taken with a CAS on WriterLockState and the owner's pid is
stored right after; release clears the pid and then the state. A process
killed in either window leaves the lock held with pid 0, and the orphan check
returns false for pid 0, so no waiter could ever recover it (14 of 60 kills
of a process looping on lock/unlock). A waiter now clears a lock that has
had no owner at every probe for two seconds.

Readers did not probe the owner at all and waited out their whole timeout
behind a writer that had been killed. They now share the writer's probe, the
first one after one interval so a reader that waits for a live writer for
microseconds pays nothing.

The tests fail on the previous code and pass now.
The per-slot sequence is write+1 once an item is published and read+capacity
once its slot is free again. With one slot those are the same number, so the
second enqueue saw a full queue as empty, overwrote the item and left the
queue unable to deliver anything (enqueue(1) and enqueue(2) both returned
true, every dequeue after that false).

ConcurrentQueue<T> and ConcurrentMessageQueue now raise a requested capacity
of 1 to 2 and reject a stored capacity below 2.
On Linux a new region is a sparse file in /dev/shm. SetLength succeeds even
when the filesystem cannot hold it, and the process is killed with SIGBUS
(not catchable) by the first write that does not fit. Docker's default
/dev/shm is 64 MB, so this is easy to hit.

The creator now compares the size with the free space of the filesystem and
throws IOException with a hint about --shm-size; the explicit FilePath mode
gets the same check for the bytes it has to add. A filesystem that cannot
report its free space is not blocked.
…paces

Containers that share /dev/shm have different PID namespaces. A waiter
looked the owner's pid up in its own namespace, did not find it (or found an
unrelated process) and either took the lock from a live owner (14 ms in a
test with unshare --pid) or never recovered a dead one.

The owner records the inode of /proc/self/ns/pid in the reserved part of the
region header, before its pid and cleared with the other owner fields. A
waiter whose namespace differs from the recorded one skips the pid-based
liveness check; the opt-in OrphanLockTimeout still applies. Locks without a
recorded namespace (earlier versions, other platforms) keep the pid-only
decision.
- StructuredMemory.AcquireWriteLock with a read guard held on the thread
  set the writer flag and waited for the thread's own read lock, which
  blocked every other process until the timeout (forever with an infinite
  timeout). It throws InvalidOperationException immediately now, as the
  automatic write lock and SharedArray already did.
- SingleProducerByteStream.Available and .Used had no disposed check and
  read the unmapped header.
- The timeout overloads of ConcurrentQueue and ConcurrentMessageQueue
  incremented the failure counters on every poll. A call counts as one
  failure when it gives up. Polling also uses a timestamp instead of a
  Stopwatch allocation.
…dren

- WaitForExit(timeout) returns before the asynchronous output readers have
  delivered the last lines, so assertions on the child's output could see an
  empty string (2 of 550 runs in a loop). A second WaitForExit() flushes
  them, and a timeout now reports what the child had printed.
- A missing worker DLL was Assert.Ignore, which would turn every
  cross-process test, the only ones that prove the lock recovery, into a
  silent skip on CI. It fails when CI is set.
- ReadLine on a lock holder had no timeout, and NUnit's [Timeout] cannot
  abort a blocked thread on .NET Core. The wait is bounded and the failure
  includes the child's stderr.
- hold_* children slept forever, so a parent that died (crash, hang
  timeout, Ctrl-C) left them holding the lock. They end when their stdin
  closes, with a two minute bound.
- The test that races Dispose against busy-polling threads moved into a
  child process (dispose_busy_poll). When the grace period loses its race
  the AccessViolationException ended the whole test host and took every
  other result with it (8 of 12 runs on 2 CPUs under load). It is tagged
  TimingSensitive because it is probabilistic.
Task.Run puts every spinning loop on a thread-pool worker. The pool starts
with one worker per core and adds one about every half second, so on a
small machine loops queued behind the first few start late or after the
test's own cancellation timer, and the result depended on test order.
Alone on 2 CPUs: Fairness_WriterUnderModerateReaders had a writer that
never ran, the 16-thread Barrier test took 10 to 13 s, and the MPMC
fairness test saw its consumer never run (producer counts [256, 0, 0, ...]).

DedicatedThread.Run uses TaskCreationOptions.LongRunning. All of them pass
alone on 2 CPUs now. The MPMC fairness ratio itself is a scheduling effect
on a machine with fewer cores than threads (thousands to one on 2 CPUs), so
it stays TimingSensitive with the reason written next to it.
continue-on-error reports a failed step as a success, so the timing
sensitive step had been failing on every run while the PR showed green. A
following step now adds a warning annotation and a section to the run
summary when it failed. The step is skipped when the build failed.

cancel-in-progress applies to pull requests only: consecutive pushes to main
used to cancel the runs of the commits in between.
- OpenExisting that arrived between the creator making the backing file and
  writing its header reported an empty file or a zero magic number as
  corrupt (13 of 300 races). It waits up to two seconds for a region that
  is still being created.
- Linux: both processes of a creation race saw an empty file and took the
  creator's role; the one that failed later (141 of 300 races with two
  capacities) unlinked the file the other used, splitting one name into two
  regions. FileMode.CreateNew decides who creates and sizes the file, and
  only that process unlinks it on failure.
- FilePath mode let MemoryMappedFile.CreateFromFile grow an existing file
  to the requested capacity before anything checked it, so a wrong
  capacity left the file permanently resized. The file is opened here with
  sharing, a size mismatch is rejected without touching it, and an older
  format or foreign file is reported as that instead of as a size problem.
- GetMemory returned a Memory<byte> whose manager did not reference the
  region, so dropping the region let the finalizer unmap it under the
  memory. The manager now holds its owner.
- Linux: a killed lock owner that its parent had not reaped is a zombie;
  GetProcessById and HasExited call it alive, so its lock was never
  recovered. /proc/<pid>/stat state Z or X now counts as gone.

Each new test fails on the previous code and passes now.
A guard knew only whether it had taken the region lock, so disposing a copy
of it decremented the thread's depth a second time. A thread inside an outer
lock then believed it held none, and the next automatic lock tried to take
the lock it already held (timeout, or for good with an infinite timeout).
Releasing a write guard while a read guard taken inside it was open removed
the region lock under that read guard.

The guards of StructuredMemory and SharedArray now carry the depth at which
they were taken. A release at another depth, or of a write lock under an
open read guard, throws SynchronizationLockException before it changes
anything, and the guard stays valid so it can be released properly. The
cached depth delegates of StructuredMemory are gone, the guard holds its
owner instead.

Dispose of either class with a guard still open on the calling thread left
the cross-process write lock held until the process ended; its owner is
alive, so no waiter recovered it. Dispose releases the calling thread's lock,
and a guard that outlives the instance disposes quietly.
OpenExisting compared the region size with the schema size before looking
at the stored version, so no SchemaCompatibility mode could ever open a
region of another size: Forward and Full only worked when an appended field
happened to fit in the 64 byte rounding, and Strict reported a size
mismatch instead of the version mismatch it exists to report. The
existing Strict test passed for that wrong reason.

The version is checked first. For different versions the region must only be
at least as large as the schema, so an older schema can read the larger
region of a newer one (it uses its first fields); a region smaller than the
schema is rejected at open time. The same version still needs the exact size.
SchemaCompatibility documents what is and is not checked, and the README has
a short section on it.
FillCore staged the value in a T[] (stackalloc or ArrayPool<T>). A managed
array cannot hold elements of 64 KiB or more, and a method that mentions
T[] or ArrayPool<T> for such a T fails to load at all, so Fill and Clear
threw TypeLoadException before writing anything. Independently, the batch
was up to 4096 elements whatever their size: 128 MiB of temporary buffer per
call for 32 KiB elements.

Elements of 32 KiB or more are written one by one from a method that has
no array in it; smaller ones are staged in batches of at most 64 KiB.
A process that dies between claiming a slot of ConcurrentQueue or
ConcurrentMessageQueue and publishing it leaves the slot claimed for good.
Nothing can recover that automatically, so the README (Recovering after a
crash) and the XML docs of both classes say so and name Remove as the way
out. SingleProducerQueue and SingleProducerByteStream are not affected.

3.0.0 takes the write lock from every live owner on Linux, including one that
runs a later version. The README and the CHANGELOG tell users to update all
processes that share a region together.
The Fill/Clear test for 100 KB elements overflowed the stack on Windows and
took the whole test host with it (380 tests had run, the rest were lost).
Every temporary copy of a 100 KB struct is 100 KB of stack and the test frame
held twelve of them, one per array[i].Tag expression and Fill argument,
more than the 1 MiB of a Windows thread. Linux threads have 8 MiB, which is
why nothing showed there. Reproduced locally with ulimit -s 1024.

The test uses the smallest size that a managed array cannot hold (64 KiB) and
reads and fills through NoInlining helpers, so no frame holds more than one
or two copies. SharedArray passes the fill value by reference from Fill down
to the copy instead of copying it into every frame.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants