Skip to content

Commit 1a1103d

Browse files
paulirwinclaude
andauthored
Fix concurrent-extension race in MMapDirectory.Map, #1090 (#1263)
* Fix concurrent-extension race in MMapDirectory.Map, #1090 When a file is being appended to concurrently (e.g. by an IndexWriter that still holds a write handle), MemoryMappedFile.CreateFromFile can throw ArgumentOutOfRangeException("capacity") because its internal stat observes a file size greater than the capacity we computed from fc.Length moments earlier. Take the max of the caller-supplied length and a fresh fc.Length read as the capacity, and retry with an updated length on that specific failure. Adds test-only counters and a stress-based regression test in TestMultiMMap. * Address Copilot review on #1090 MMapDirectory.Map fix Narrow the ArgumentOutOfRangeException retry filter to ParamName == "capacity" so unrelated argument errors aren't masked, and mark the regression test NonParallelizable since it relies on static counters on MMapDirectory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 36b57d5 commit 1a1103d

2 files changed

Lines changed: 167 additions & 8 deletions

File tree

src/Lucene.Net.Tests/Store/TestMultiMMap.cs

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
using Lucene.Net.Index.Extensions;
44
using NUnit.Framework;
55
using System;
6+
using System.Diagnostics;
67
using System.IO;
78
using System.Text;
9+
using System.Threading;
810
using Assert = Lucene.Net.TestFramework.Assert;
911

1012
namespace Lucene.Net.Store
@@ -415,6 +417,118 @@ private void AssertChunking(Random random, int chunkSize)
415417
}
416418

417419

420+
// LUCENENET: Regression test for GitHub #1090. A background thread
421+
// extends a file on disk while the foreground thread repeatedly
422+
// opens it with MMapDirectory.OpenInput. Before the fix, the
423+
// file's length could grow between the caller capturing fc.Length
424+
// and MemoryMappedFile.CreateFromFile performing its internal
425+
// stat, causing ArgumentOutOfRangeException (paramName="capacity")
426+
// with the message "The capacity may not be smaller than the
427+
// file size."
428+
// NonParallelizable: the retry-path assertion reads static counters on
429+
// MMapDirectory, so any other test exercising MMapDirectory in parallel
430+
// could skew the observed retry count.
431+
[Test, LuceneNetSpecific, Slow, NonParallelizable]
432+
public void TestOpenInputConcurrentFileExtension_Issue1090()
433+
{
434+
var dir = CreateTempDir("testOpenInputConcurrentFileExtension");
435+
const string name = "data.bin";
436+
string filePath = Path.Combine(dir.FullName, name);
437+
438+
// Seed with a small initial payload.
439+
File.WriteAllBytes(filePath, new byte[64]);
440+
441+
using var mmapDir = new MMapDirectory(dir);
442+
443+
const long maxFileSize = 1L * 1024 * 1024; // 1 MiB cap
444+
var stop = new ManualResetEventSlim(false);
445+
Exception writerError = null;
446+
447+
var writer = new Thread(() =>
448+
{
449+
var chunk = new byte[64];
450+
try
451+
{
452+
while (!stop.IsSet)
453+
{
454+
using var fs = new FileStream(filePath, FileMode.Open, FileAccess.Write, FileShare.ReadWrite);
455+
fs.Seek(0, SeekOrigin.End);
456+
if (fs.Length < maxFileSize)
457+
{
458+
fs.Write(chunk, 0, chunk.Length);
459+
}
460+
else
461+
{
462+
// Keep the file bounded: truncate back and grow again.
463+
fs.SetLength(64);
464+
}
465+
}
466+
}
467+
catch (Exception e)
468+
{
469+
writerError = e;
470+
}
471+
})
472+
{ IsBackground = true, Name = "mmap-issue1090-extender" };
473+
writer.Start();
474+
475+
// Snapshot counters so this test's assertion is not affected by
476+
// any earlier test's activity on MMapDirectory.
477+
long baselineRetries = Interlocked.Read(ref MMapDirectory.s_capacityRetryCount);
478+
479+
try
480+
{
481+
var sw = Stopwatch.StartNew();
482+
int iterations = 0;
483+
// Keep stretching the window until either the race fires or we
484+
// hit a hard deadline. On most machines this takes < 1 second.
485+
const int maxSeconds = 15;
486+
while (sw.Elapsed < TimeSpan.FromSeconds(maxSeconds))
487+
{
488+
using (var _ = mmapDir.OpenInput(name, NewIOContext(Random)))
489+
{
490+
// Just open and dispose; the race occurs during construction.
491+
}
492+
iterations++;
493+
494+
if (Interlocked.Read(ref MMapDirectory.s_capacityRetryCount) > baselineRetries)
495+
{
496+
break; // race reproduced and handled by the retry loop
497+
}
498+
}
499+
500+
long retries = Interlocked.Read(ref MMapDirectory.s_capacityRetryCount) - baselineRetries;
501+
int maxAttempts = Volatile.Read(ref MMapDirectory.s_maxCapacityAttemptsObserved);
502+
503+
// Surface what was observed for diagnostics when run with -v normal.
504+
TestContext.Progress.WriteLine(
505+
$"TestOpenInputConcurrentFileExtension: iterations={iterations}, retries={retries}, maxAttemptsObserved={maxAttempts}");
506+
507+
// The real check: the race must have fired and our retry loop
508+
// must have swallowed it. Without the fix, the exception
509+
// escapes OpenInput and the test fails with ArgumentOutOfRangeException
510+
// (as seen in #1090). If the race never fires during this run
511+
// (timing-dependent), mark the test inconclusive rather than
512+
// silently passing — we haven't actually exercised the fix.
513+
if (retries == 0)
514+
{
515+
NUnit.Framework.Assert.Inconclusive(
516+
$"The concurrent-extension race was not reproduced within {maxSeconds}s " +
517+
$"({iterations} OpenInput iterations). The fix was therefore not exercised on this run.");
518+
}
519+
}
520+
finally
521+
{
522+
stop.Set();
523+
writer.Join();
524+
}
525+
526+
if (writerError != null)
527+
{
528+
throw new Exception("Writer thread failed", writerError);
529+
}
530+
}
531+
418532
[Test, LuceneNetSpecific]
419533
public void TestDisposeIndexInput()
420534
{

src/Lucene.Net/Store/MMapDirectory.cs

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,15 @@ public class MMapDirectory : FSDirectory
6363

6464
private readonly int chunkSizePower;
6565

66+
// LUCENENET specific BEGIN: test-only counters for the capacity-retry
67+
// path in Map() — see #1090. Internal (exposed via InternalsVisibleTo
68+
// to the test assemblies) so regression tests can assert that the
69+
// race was actually exercised during a run, and to gather data on how
70+
// many retries are typically needed. Not intended for production use.
71+
internal static long s_capacityRetryCount;
72+
internal static int s_maxCapacityAttemptsObserved;
73+
// LUCENENET specific END
74+
6675
/// <summary>
6776
/// Create a new <see cref="MMapDirectory"/> for the named location.
6877
/// </summary>
@@ -311,16 +320,52 @@ internal virtual ByteBuffer[] Map(MMapIndexInput input, FileStream fc, long offs
311320

312321
if (input.memoryMappedFile is null)
313322
{
314-
input.memoryMappedFile = MemoryMappedFile.CreateFromFile(
315-
fileStream: fc,
316-
mapName: null,
317-
capacity: length,
318-
access: MemoryMappedFileAccess.Read,
323+
// LUCENENET specific BEGIN: MemoryMappedFile.CreateFromFile
324+
// performs an internal stat and throws
325+
// ArgumentOutOfRangeException("capacity") if the on-disk file
326+
// size exceeds the requested capacity. When another
327+
// process/thread is appending to this file (e.g. an
328+
// IndexWriter that still holds a write handle), the file can
329+
// grow between when we capture fc.Length and when
330+
// CreateFromFile reads the size. Retry with the latest
331+
// observed length on that specific failure; we only map the
332+
// bytes the caller requested via the buffer-sizing loop
333+
// below, so an oversized capacity is harmless. See #1090.
334+
long capacity = Math.Max(length, fc.Length);
335+
const int maxAttempts = 5;
336+
int attempt = 0;
337+
while (true)
338+
{
339+
try
340+
{
341+
input.memoryMappedFile = MemoryMappedFile.CreateFromFile(
342+
fileStream: fc,
343+
mapName: null,
344+
capacity: capacity,
345+
access: MemoryMappedFileAccess.Read,
319346
#if FEATURE_MEMORYMAPPEDFILESECURITY
320-
memoryMappedFileSecurity: null,
347+
memoryMappedFileSecurity: null,
321348
#endif
322-
inheritability: HandleInheritability.Inheritable,
323-
leaveOpen: true); // LUCENENET: We explicitly dispose the FileStream separately.
349+
inheritability: HandleInheritability.Inheritable,
350+
leaveOpen: true); // LUCENENET: We explicitly dispose the FileStream separately.
351+
break;
352+
}
353+
catch (ArgumentOutOfRangeException e) when (e.ParamName == "capacity" && attempt < maxAttempts - 1)
354+
{
355+
Interlocked.Increment(ref s_capacityRetryCount);
356+
capacity = Math.Max(capacity, fc.Length);
357+
attempt++;
358+
}
359+
}
360+
// Record the highest total attempts observed (1 = first try succeeded).
361+
int attemptsTaken = attempt + 1;
362+
int prior;
363+
do
364+
{
365+
prior = Volatile.Read(ref s_maxCapacityAttemptsObserved);
366+
if (attemptsTaken <= prior) break;
367+
} while (Interlocked.CompareExchange(ref s_maxCapacityAttemptsObserved, attemptsTaken, prior) != prior);
368+
// LUCENENET specific END
324369
}
325370

326371
long bufferStart = 0L;

0 commit comments

Comments
 (0)