Fix capacity race on .NET Framework

Author: paulirwin

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

@@ -498,14 +498,31 @@ private void AssertChunking(Random random, int chunkSize)
         // extends a file on disk while the foreground thread repeatedly
         // opens it with MMapDirectory.OpenInput. The original failure
         // mode was ArgumentOutOfRangeException(paramName="capacity")
-        // from MemoryMappedFile.CreateFromFile, because we were passing
-        // a caller-computed capacity (fc.Length) that could be smaller
-        // than the actual file size by the time the framework did its
-        // internal stat. The current design passes capacity: 0, which
-        // tells the framework to size the mapping from the file's
-        // current on-disk length atomically — there is no caller-side
-        // capacity to disagree with the file. This test asserts that
-        // OpenInput continues to succeed under concurrent file extension.
+        // from MemoryMappedFile.CreateFromFile, because the on-disk file
+        // size could exceed our caller-computed capacity by the time the
+        // framework did its internal stat. .NET Framework's
+        // CreateFromFile reads fileStream.Length multiple times
+        // non-atomically (referencesource MemoryMappedFile.cs L192-L243);
+        // modern .NET snapshots it into a single local
+        // (dotnet/runtime MemoryMappedFile.cs L237-L268). Even when we
+        // pass capacity: 0 the .NET Framework path still races because
+        // the length is re-read for both the defaulting step and the
+        // capacity-vs-size guard. SharedMapping.Create handles the
+        // residual race with a retry loop. This test asserts that
+        // OpenInput continues to succeed under concurrent file
+        // extension.
+        //
+        // Test design notes:
+        // - The writer extends only (never truncates). Truncating a
+        //   user-mapped file on Windows fails with ERROR_USER_MAPPED_FILE
+        //   and is unrelated to what we're verifying here.
+        // - The reader runs a bounded number of iterations rather than a
+        //   wall-clock loop. Sustained mmap churn (thousands of
+        //   map/unmap pairs per second) can transiently exhaust Windows
+        //   kernel resources (ERROR_NO_SYSTEM_RESOURCES,
+        //   ERROR_ACCESS_DENIED on view creation), which is also
+        //   unrelated to the capacity race. A few hundred iterations
+        //   are plenty to repeatedly hit the race window.
         [Test, LuceneNetSpecific, Slow]
         public void TestOpenInputConcurrentFileExtension_Issue1090()
         {
@@ -518,7 +535,7 @@ public void TestOpenInputConcurrentFileExtension_Issue1090()
 
             using var mmapDir = new MMapDirectory(dir);
 
-            const long maxFileSize = 1L * 1024 * 1024; // 1 MiB cap
+            const long maxFileSize = 64L * 1024 * 1024; // 64 MiB safety cap
             var stop = new ManualResetEventSlim(false);
             Exception writerError = null;
 
@@ -531,15 +548,14 @@ public void TestOpenInputConcurrentFileExtension_Issue1090()
                     {
                         using var fs = new FileStream(filePath, FileMode.Open, FileAccess.Write, FileShare.ReadWrite);
                         fs.Seek(0, SeekOrigin.End);
-                        if (fs.Length < maxFileSize)
-                        {
-                            fs.Write(chunk, 0, chunk.Length);
-                        }
-                        else
+                        if (fs.Length >= maxFileSize)
                         {
-                            // Keep the file bounded: truncate back and grow again.
-                            fs.SetLength(64);
+                            // Stop extending if we somehow reach the cap. The
+                            // reader's bounded iteration count guarantees this
+                            // is far above what we'll hit in a normal run.
+                            break;
                         }
+                        fs.Write(chunk, 0, chunk.Length);
                     }
                 }
                 catch (Exception e)
@@ -552,23 +568,17 @@ public void TestOpenInputConcurrentFileExtension_Issue1090()
 
             try
             {
-                var sw = Stopwatch.StartNew();
-                int iterations = 0;
-                // Stress OpenInput while the background thread extends/truncates the
-                // file. Each call must succeed cleanly. A short window is plenty:
-                // before the capacity:0 fix this race fired in well under a second.
-                const int maxSeconds = 5;
-                while (sw.Elapsed < TimeSpan.FromSeconds(maxSeconds))
+                // Bounded iteration count keeps mmap churn well below the
+                // Windows kernel-resource threshold while still exercising
+                // the capacity race many times over.
+                const int iterations = 500;
+                for (int i = 0; i < iterations; i++)
                 {
                     using (var _ = mmapDir.OpenInput(name, NewIOContext(Random)))
                     {
-                        // Just open and dispose; the race occurred during construction.
+                        // Just open and dispose; the race occurs during construction.
                     }
-                    iterations++;
                 }
-
-                TestContext.Progress.WriteLine(
-                    $"TestOpenInputConcurrentFileExtension: completed {iterations} OpenInput calls in {sw.Elapsed.TotalSeconds:F1}s");
             }
             finally
             {

src/Lucene.Net/Store/MMapDirectory.cs

@@ -839,6 +839,53 @@ private SharedMapping(MemoryMappedFile? mmf, Chunk[] chunks, long length)
             }
 
             internal static SharedMapping Create(string file, int chunkSizePower)
+            {
+                // .NET Framework's MemoryMappedFile.CreateFromFile reads
+                // fileStream.Length multiple times non-atomically: once
+                // to materialize a default capacity (when 0 is passed),
+                // then again to enforce `fileStream.Length <= capacity`.
+                // A concurrent extender that grows the file between
+                // those reads trips an ArgumentOutOfRangeException
+                // ("capacity") with message "The capacity may not be
+                // smaller than the file size." See referencesource
+                // System.Core/System/IO/MemoryMappedFiles/
+                // MemoryMappedFile.cs lines 192-243:
+                // https://github.com/microsoft/referencesource/blob/ec9fa9ae770d522a5b5f0607898044b7478574a3/System.Core/System/IO/MemoryMappedFiles/MemoryMappedFile.cs#L192-L243
+                //
+                // Modern .NET (dotnet/runtime) caches the length into a
+                // single local fileSize at the top of CreateFromFile
+                // and reuses it for both the defaulting step and the
+                // VerifyMemoryMappedFileAccess guard, so the race
+                // cannot fire and this loop runs once:
+                // https://github.com/dotnet/runtime/blob/550500a978b784658a04110d49b3335dcacf33e0/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs#L237-L268
+                // https://github.com/dotnet/runtime/blob/550500a978b784658a04110d49b3335dcacf33e0/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Windows.cs#L14-L26
+                //
+                // The retry budget is generous because the race window
+                // is small but the retry is cheap (a FileStream reopen
+                // plus another CreateFromFile call), and a tight
+                // extender can keep losing the race for many attempts
+                // in a row. Yield between attempts so the extender
+                // thread can make progress and reach a stable point
+                // between writes. (#1090)
+                const int maxAttempts = 32;
+                for (int attempt = 0; ; attempt++)
+                {
+                    try
+                    {
+                        return CreateAttempt(file, chunkSizePower);
+                    }
+                    catch (ArgumentOutOfRangeException e)
+                        when (e.ParamName == "capacity" && attempt < maxAttempts - 1)
+                    {
+                        // Re-open and retry. The FileStream from the failed
+                        // attempt was disposed by CreateFromFile (leaveOpen:
+                        // false) before the exception propagated.
+                        Thread.Yield();
+                    }
+                }
+            }
+
+            private static SharedMapping CreateAttempt(string file, int chunkSizePower)
             {
                 // We open our own FileStream so we control the FileShare
                 // flags. The path-based CreateFromFile overload internally
@@ -878,13 +925,16 @@ internal static SharedMapping Create(string file, int chunkSizePower)
                         return new SharedMapping(mmf: null, chunks: Array.Empty<Chunk>(), length: 0);
                     }
 
-                    // capacity: 0 -> the framework uses the file's
-                    // current size on disk, atomically with mapping
-                    // creation. This eliminates the #1090 race window
-                    // we previously had to retry around.
-                    // leaveOpen: false -> the MMF takes ownership of the
-                    // FileStream and disposes it on its own Dispose, so
-                    // we don't need to track it ourselves.
+                    // capacity: 0 -> the framework sizes the mapping
+                    // from the file's current length. On modern .NET
+                    // that length is captured into a single local and
+                    // reused, so there is no race; on .NET Framework
+                    // the length is re-read across the defaulting and
+                    // validation steps, which is why Create wraps this
+                    // call in a retry loop.
+                    // leaveOpen: false -> the MMF takes ownership of
+                    // the FileStream and disposes it on its own
+                    // Dispose, so we don't need to track it ourselves.
                     mmf = MemoryMappedFile.CreateFromFile(
                         fileStream: fs,
                         mapName: null,