Skip to content

Commit 9dfe8bb

Browse files
paulirwinclaude
andcommitted
Fix cross-thread Dispose NRE: do not null readBase under a live reader
TestCrossThreadCloneDisposeWhileReading crashed the test host on net48 x64 with a NullReferenceException in ReadByte at *(readBase + pos): a clone being read on thread A was disposed on thread B, and Dispose called ReleaseCurrentChunk(), nulling readBase while thread A sat between its `pos < currentEnd` check and its dereference. Dispose now only zeroes currentEnd (the fail-fast gate) and no longer nulls readBase. There is no native reference tied to the cursor (the DrainReclaimer owns reclamation, and keeps the view mapped until the owning root closes), so a stale readBase is harmless: the in-flight read completes against the still-mapped view, and the zeroed currentEnd makes every subsequent read drop to the slow path and throw AlreadyClosed. readBase is now only ever nulled by the reading thread itself (Seek / EnsureCurrentChunk / clone setup), never cross-thread, so the race is structurally eliminated. The fields are GC'd with the instance. Latent on net10 (timing-lucky in CI); manifested as a host crash on net48 x64. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d853806 commit 9dfe8bb

1 file changed

Lines changed: 13 additions & 7 deletions

File tree

src/Lucene.Net/Support/UnsafeChunkIndexInput.cs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -503,17 +503,23 @@ protected override void Dispose(bool disposing)
503503
{
504504
return;
505505
}
506-
// Clear currentEnd so a racing reader's fast path (`pos < currentEnd`)
506+
// Clear currentEnd so a future read's fast path (`pos < currentEnd`)
507507
// drops to the slow path and observes instanceClosed. Interlocked so the
508508
// 64-bit write can't tear on 32-bit runtimes.
509+
//
510+
// We deliberately do NOT null readBase here. Disposing an instance while
511+
// another thread reads it (e.g. a slicer cascade disposing a slice that
512+
// thread is mid-read, or a cross-thread clone Dispose) is racy: that
513+
// reader may sit between its `pos < currentEnd` check and its
514+
// `*(readBase + pos)` load, so nulling readBase under it would NRE/AVE.
515+
// There is no native reference tied to the cursor (the reclaimer owns
516+
// reclamation), so leaving readBase stale is harmless - the zeroed
517+
// currentEnd already fails-fast every subsequent read, and the in-flight
518+
// read completes safely against the still-mapped view (the reclaimer
519+
// keeps it mapped until the owning root closes). The fields are GC'd with
520+
// the instance.
509521
Interlocked.Exchange(ref currentEnd, 0L);
510522

511-
// Invalidate this instance's cursor cache. This only affects this input;
512-
// the shared mapping (and its reclaimer) is closed by the owning root's
513-
// DisposeChunkSource, NOT here, so disposing a clone or slice never tears
514-
// down the mapping that sibling readers still depend on.
515-
ReleaseCurrentChunk();
516-
517523
DisposeChunkSource(disposing);
518524
}
519525

0 commit comments

Comments
 (0)