Fix FSDirectory fsync race on output close

src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs

@@ -47,7 +47,6 @@ public class TestIndexWriterOnJRECrash : TestNRTThreads
 
         [Test]
         [Slow]
-        [AwaitsFix]
         public override void TestNRTThreads_Mem()
         {
             //if we are not the fork
@@ -315,25 +314,39 @@ public virtual bool CheckIndexes(FileSystemInfo file)
         {
             if (file is DirectoryInfo directoryInfo)
             {
-                BaseDirectoryWrapper dir = NewFSDirectory(directoryInfo);
-                dir.CheckIndexOnDispose = false; // don't double-checkindex
-                if (DirectoryReader.IndexExists(dir))
+                BaseDirectoryWrapper dir = null;
+                Exception priorE = null;
+                try
                 {
-                    if (Verbose)
-                    {
-                        Console.Error.WriteLine("Checking index: " + file);
-                    }
-                    // LUCENE-4738: if we crashed while writing first
-                    // commit it's possible index will be corrupt (by
-                    // design we don't try to be smart about this case
-                    // since that too risky):
-                    if (SegmentInfos.GetLastCommitGeneration(dir) > 1)
+                    dir = NewFSDirectory(directoryInfo);
+                    dir.CheckIndexOnDispose = false; // don't double-checkindex
+                    if (DirectoryReader.IndexExists(dir))
                     {
-                        TestUtil.CheckIndex(dir);
+                        if (Verbose)
+                        {
+                            Console.Error.WriteLine("Checking index: " + file);
+                        }
+                        // LUCENE-4738: if we crashed while writing first
+                        // commit it's possible index will be corrupt (by
+                        // design we don't try to be smart about this case
+                        // since that too risky):
+                        if (SegmentInfos.GetLastCommitGeneration(dir) > 1)
+                        {
+                            TestUtil.CheckIndex(dir);
+                        }
+                        return true;
                     }
-                    return true;
                 }
-                dir.Dispose();
+                catch (Exception e)
+                {
+                    priorE = e;
+                    throw;
+                }
+                finally
+                {
+                    IOUtils.DisposeWhileHandlingException(priorE, dir);
+                }
+
                 foreach (DirectoryInfo f in directoryInfo.EnumerateDirectories())
                 {
                     if (CheckIndexes(f))

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

@@ -438,6 +438,41 @@ public virtual void TestFsyncDoesntCreateNewFiles()
             Assert.AreEqual(0, fsdir.ListAll().Length);
         }
 
+        [Test]
+        [LuceneNetSpecific]
+        public virtual void TestFSIndexOutputFlushesBeforeMarkedStale()
+        {
+            DirectoryInfo path = CreateTempDir("flushBeforeMarkedStale");
+            using var dir = new FlushObservingSimpleFSDirectory(path);
+
+            byte[] bytes = new byte[1];
+            Random.NextBytes(bytes);
+
+            using (IndexOutput output = dir.CreateOutput("afile", NewIOContext(Random)))
+            {
+                output.WriteBytes(bytes, bytes.Length);
+            }
+
+            Assert.AreEqual(bytes.Length, dir.LengthObservedOnIndexOutputClosed,
+                "FSDirectory must flush FileStream's managed buffer before marking a file stale for fsync.");
+        }
+
+        private sealed class FlushObservingSimpleFSDirectory : SimpleFSDirectory
+        {
+            public FlushObservingSimpleFSDirectory(DirectoryInfo path)
+                : base(path)
+            {
+            }
+
+            public long LengthObservedOnIndexOutputClosed { get; private set; } = -1;
+
+            protected override void OnIndexOutputClosed(FSIndexOutput io)
+            {
+                LengthObservedOnIndexOutputClosed = new FileInfo(Path.Combine(m_directory.FullName, io.name)).Length;
+                base.OnIndexOutputClosed(io);
+            }
+        }
+
         [Test]
         [Slow]
         [LuceneNetSpecific]

src/Lucene.Net/Store/FSDirectory.cs

@@ -574,27 +574,48 @@ public override void Flush()
             /// <inheritdoc/>
             protected override void Dispose(bool disposing)
             {
-                if (disposing)
+                if (disposing && isOpen)
                 {
-                    parent.OnIndexOutputClosed(this);
-                    // only close the file if it has not been closed yet
-                    if (isOpen)
+                    Exception priorE = null; // LUCENENET: No need to cast to IOException
+
+                    try
                     {
-                        Exception priorE = null; // LUCENENET: No need to cast to IOException
+                        // LUCENENET: FileStream has a managed buffer. If we add this file
+                        // to m_staleFiles before flushing that buffer, another thread can
+                        // fsync the file through a separate handle, remove the stale marker,
+                        // and then this FileStream can flush later. That leaves bytes written
+                        // after the fsync and can make a committed index non-durable after a
+                        // process or machine crash.
+                        //
+                        // Keep the FileStream flush and stale-file bookkeeping atomic with
+                        // FSDirectory.Sync() so a file is never considered synced until all
+                        // managed FileStream buffers have at least reached the OS.
+                        UninterruptableMonitor.Enter(parent.m_syncLock);
                         try
                         {
-                            file.Flush(flushToDisk: false);
-                        }
-                        catch (Exception ioe) when (ioe.IsIOException())
-                        {
-                            priorE = ioe;
+                            try
+                            {
+                                file.Flush(flushToDisk: false);
+                            }
+                            catch (Exception ioe) when (ioe.IsIOException())
+                            {
+                                priorE = ioe;
+                            }
+                            finally
+                            {
+                                parent.OnIndexOutputClosed(this);
+                            }
                         }
                         finally
                         {
-                            isOpen = false;
-                            IOUtils.DisposeWhileHandlingException(priorE, file);
+                            UninterruptableMonitor.Exit(parent.m_syncLock);
                         }
                     }
+                    finally
+                    {
+                        isOpen = false;
+                        IOUtils.DisposeWhileHandlingException(priorE, file);
+                    }
                 }
                 //base.Dispose(disposing); // LUCENENET: No need to call base class, we are not using the functionality of BufferedIndexOutput
             }