From b2b8cdcafb07b60ca8aad47b6be06984ebc78182 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Fri, 10 May 2024 21:41:01 -0600 Subject: [PATCH 01/17] Restore fsync behavior in FSDirectory via P/Invoke This restores the commented-out fsync behavior in FSDirectory to help mitigate a performance regression in .NET 8. --- src/Lucene.Net/Store/FSDirectory.cs | 66 +++++------ src/Lucene.Net/Support/ConcurrentHashSet.cs | 25 ++-- .../Support/IO/PosixFsyncSupport.cs | 95 +++++++++++++++ .../Support/IO/WindowsFsyncSupport.cs | 110 ++++++++++++++++++ src/Lucene.Net/Util/IOUtils.cs | 54 ++++++++- 5 files changed, 300 insertions(+), 50 deletions(-) create mode 100644 src/Lucene.Net/Support/IO/PosixFsyncSupport.cs create mode 100644 src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs diff --git a/src/Lucene.Net/Store/FSDirectory.cs b/src/Lucene.Net/Store/FSDirectory.cs index 2439b8a92b..746c0309c6 100644 --- a/src/Lucene.Net/Store/FSDirectory.cs +++ b/src/Lucene.Net/Store/FSDirectory.cs @@ -67,11 +67,11 @@ namespace Lucene.Net.Store /// the best implementation given your /// environment, and the known limitations of each /// implementation. For users who have no reason to prefer a - /// specific implementation, it's best to simply use + /// specific implementation, it's best to simply use /// (or one of its overloads). For all others, you should instantiate the /// desired implementation directly. /// - /// The locking implementation is by default + /// The locking implementation is by default /// , but can be changed by /// passing in a custom instance. /// @@ -94,10 +94,8 @@ public abstract class FSDirectory : BaseDirectory public const int DEFAULT_READ_CHUNK_SIZE = 8192; protected readonly DirectoryInfo m_directory; // The underlying filesystem directory + protected readonly ISet m_staleFiles = new ConcurrentHashSet(); // Files written, but not yet sync'ed - // LUCENENET specific: No such thing as "stale files" in .NET, since Flush(true) writes everything to disk before - // our FileStream is disposed. - //protected readonly ISet m_staleFiles = new ConcurrentHashSet(); // Files written, but not yet sync'ed #pragma warning disable 612, 618 private int chunkSize = DEFAULT_READ_CHUNK_SIZE; #pragma warning restore 612, 618 @@ -321,9 +319,8 @@ public override void DeleteFile(string name) { throw new IOException("Cannot delete " + file, e); } - // LUCENENET specific: No such thing as "stale files" in .NET, since Flush(true) writes everything to disk before - // our FileStream is disposed. - //m_staleFiles.Remove(name); + + m_staleFiles.Remove(name); } /// @@ -366,35 +363,29 @@ protected virtual void EnsureCanWrite(string name) protected virtual void OnIndexOutputClosed(FSIndexOutput io) { - // LUCENENET specific: No such thing as "stale files" in .NET, since Flush(true) writes everything to disk before - // our FileStream is disposed. - //m_staleFiles.Add(io.name); + m_staleFiles.Add(io.name); } public override void Sync(ICollection names) { EnsureOpen(); - // LUCENENET specific: No such thing as "stale files" in .NET, since Flush(true) writes everything to disk before - // our FileStream is disposed. Therefore, there is nothing else to do in this method. - //ISet toSync = new HashSet(names); - //toSync.IntersectWith(m_staleFiles); - - //// LUCENENET specific: Fsync breaks concurrency here. - //// Part of a solution suggested by Vincent Van Den Berghe: http://apache.markmail.org/message/hafnuhq2ydhfjmi2 - ////foreach (var name in toSync) - ////{ - //// Fsync(name); - ////} - - //// fsync the directory itsself, but only if there was any file fsynced before - //// (otherwise it can happen that the directory does not yet exist)! - //if (toSync.Count > 0) - //{ - // IOUtils.Fsync(m_directory.FullName, true); - //} + ISet toSync = new HashSet(names); + toSync.IntersectWith(m_staleFiles); + + foreach (var name in toSync) + { + Fsync(name); + } - //m_staleFiles.ExceptWith(toSync); + // fsync the directory itsself, but only if there was any file fsynced before + // (otherwise it can happen that the directory does not yet exist)! + if (toSync.Count > 0) + { + IOUtils.Fsync(m_directory, true); + } + + m_staleFiles.ExceptWith(toSync); } public override string GetLockID() @@ -546,7 +537,7 @@ protected override void Dispose(bool disposing) Exception priorE = null; // LUCENENET: No need to cast to IOExcpetion try { - file.Flush(flushToDisk: true); + file.Flush(flushToDisk: false); } catch (Exception ioe) when (ioe.IsIOException()) { @@ -586,12 +577,9 @@ public override void Seek(long pos) public override long Position => file.Position; // LUCENENET specific - need to override, since we are buffering locally, renamed from getFilePointer() to match FileStream } - // LUCENENET specific: Fsync is pointless in .NET, since we are - // calling FileStream.Flush(true) before the stream is disposed - // which means we never need it at the point in Java where it is called. - //protected virtual void Fsync(string name) - //{ - // IOUtils.Fsync(Path.Combine(m_directory.FullName, name), false); - //} + protected virtual void Fsync(string name) + { + IOUtils.Fsync(new FileInfo(Path.Combine(m_directory.FullName, name)), false); + } } -} \ No newline at end of file +} diff --git a/src/Lucene.Net/Support/ConcurrentHashSet.cs b/src/Lucene.Net/Support/ConcurrentHashSet.cs index dd79ead7f0..917b3ce356 100644 --- a/src/Lucene.Net/Support/ConcurrentHashSet.cs +++ b/src/Lucene.Net/Support/ConcurrentHashSet.cs @@ -204,16 +204,16 @@ public ConcurrentHashSet(IEnumerable collection, IEqualityComparer compare /// - /// Initializes a new instance of the - /// class that contains elements copied from the specified , - /// has the specified concurrency level, has the specified initial capacity, and uses the specified + /// Initializes a new instance of the + /// class that contains elements copied from the specified , + /// has the specified concurrency level, has the specified initial capacity, and uses the specified /// . /// - /// The estimated number of threads that will update the + /// The estimated number of threads that will update the /// concurrently. - /// The whose elements are copied to the new + /// The whose elements are copied to the new /// . - /// The implementation to use + /// The implementation to use /// when comparing items. /// /// is a null reference. @@ -638,7 +638,7 @@ private void GrowTable(Tables tables) // We want to make sure that GrowTable will not be called again, since table is at the maximum size. // To achieve that, we set the budget to int.MaxValue. // - // (There is one special case that would allow GrowTable() to be called in the future: + // (There is one special case that would allow GrowTable() to be called in the future: // calling Clear() on the ConcurrentHashSet will shrink the table and lower the budget.) _budget = int.MaxValue; } @@ -753,7 +753,16 @@ private void CopyToItems(T[] array, int index) public void ExceptWith(IEnumerable other) { - throw new NotImplementedException(); + if (ReferenceEquals(this, other)) + { + Clear(); + return; + } + + foreach (var item in other) + { + TryRemove(item); + } } public void IntersectWith(IEnumerable other) diff --git a/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs b/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs new file mode 100644 index 0000000000..e4d6c833a0 --- /dev/null +++ b/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs @@ -0,0 +1,95 @@ +using Lucene.Net.Util; +using System.IO; +using System.Runtime.InteropServices; + +namespace Lucene.Net.Support.IO +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + internal static class PosixFsyncSupport + { + // https://pubs.opengroup.org/onlinepubs/009695399/functions/fsync.html + [DllImport("libc", SetLastError = true)] + private static extern int fsync(int fd); + + // https://pubs.opengroup.org/onlinepubs/007904875/functions/open.html + [DllImport("libc", SetLastError = true)] + private static extern int open([MarshalAs(UnmanagedType.LPStr)] string pathname, int flags); + + // https://pubs.opengroup.org/onlinepubs/009604499/functions/close.html + [DllImport("libc", SetLastError = true)] + private static extern int close(int fd); + + // https://pubs.opengroup.org/onlinepubs/007904975/functions/fcntl.html + // and https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fcntl.2.html + [DllImport("libc", SetLastError = true)] + private static extern int fcntl(int fd, int cmd, int arg); + + private const int O_RDONLY = 0; + private const int O_WRONLY = 1; + + // https://opensource.apple.com/source/xnu/xnu-6153.81.5/bsd/sys/fcntl.h.auto.html + private const int F_FULLFSYNC = 51; + + public static void Fsync(string path, bool isDir) + { + using DescriptorWrapper handle = new DescriptorWrapper(path, isDir); + handle.Flush(); + } + + private readonly ref struct DescriptorWrapper + { + private readonly int fd; + + public DescriptorWrapper(string path, bool isDir) + { + fd = open(path, isDir ? O_RDONLY : O_WRONLY); + + if (fd == -1) + { + int error = Marshal.GetLastWin32Error(); + throw new IOException($"Unable to open path, error: 0x{error:x8}", error); + } + } + + public void Flush() + { + // if macOS, use F_FULLFSYNC + if (Constants.MAC_OS_X) + { + if (fcntl(fd, F_FULLFSYNC, 0) == -1) + { + throw new IOException("fcntl failed", Marshal.GetLastWin32Error()); + } + } + else if (fsync(fd) == -1) + { + throw new IOException("fsync failed", Marshal.GetLastWin32Error()); + } + } + + public void Dispose() + { + if (close(fd) == -1) + { + throw new IOException("close failed", Marshal.GetLastWin32Error()); + } + } + } + } +} diff --git a/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs b/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs new file mode 100644 index 0000000000..09666b88a7 --- /dev/null +++ b/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs @@ -0,0 +1,110 @@ +using System; +using System.IO; +using System.Runtime.InteropServices; + +namespace Lucene.Net.Support.IO +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + public static class WindowsFsyncSupport + { + // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern IntPtr CreateFileW( + string lpFileName, + uint dwDesiredAccess, + uint dwShareMode, + IntPtr lpSecurityAttributes, + uint dwCreationDisposition, + uint dwFlagsAndAttributes, + IntPtr hTemplateFile + ); + + // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-flushfilebuffers + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool FlushFileBuffers(IntPtr hFile); + + // https://learn.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-closehandle + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool CloseHandle(IntPtr hObject); + + private static readonly IntPtr INVALID_HANDLE_VALUE = new IntPtr(-1); + + private const int ERROR_ACCESS_DENIED = 5; + + // https://learn.microsoft.com/en-us/windows/win32/secauthz/generic-access-rights + private const int GENERIC_WRITE = 0x40000000; + + private const int FILE_SHARE_READ = 0x00000001; + private const int FILE_SHARE_WRITE = 0x00000002; + private const int FILE_SHARE_DELETE = 0x00000004; + private const int FILE_FLAG_BACKUP_SEMANTICS = 0x02000000; + private const int OPEN_EXISTING = 3; + + public static void Fsync(string path, bool isDir) + { + using HandleWrapper handle = new HandleWrapper(path, isDir); + handle.Flush(); + } + + private readonly ref struct HandleWrapper + { + private readonly IntPtr handle; + + public HandleWrapper(string path, bool isDir) + { + handle = CreateFileW(path, + GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + IntPtr.Zero, + OPEN_EXISTING, + (uint)(isDir ? FILE_FLAG_BACKUP_SEMANTICS : 0), // FILE_FLAG_BACKUP_SEMANTICS required to open a directory + IntPtr.Zero); + + if (handle == INVALID_HANDLE_VALUE) + { + int error = Marshal.GetLastWin32Error(); + throw new IOException($"Unable to open directory, error: 0x{error:x8}", error); + } + } + + public void Flush() + { + if (!FlushFileBuffers(handle)) + { + int error = Marshal.GetLastWin32Error(); + + if (error != ERROR_ACCESS_DENIED) + { + // swallow ERROR_ACCESS_DENIED like in OpenJDK + throw new IOException($"FlushFileBuffers failed, error: 0x{error:x8}", error); + } + } + } + + public void Dispose() + { + if (!CloseHandle(handle)) + { + int error = Marshal.GetLastWin32Error(); + throw new IOException($"CloseHandle failed, error: 0x{error:x8}", error); + } + } + } + } +} diff --git a/src/Lucene.Net/Util/IOUtils.cs b/src/Lucene.Net/Util/IOUtils.cs index bbfdb1ccfa..74f58c396d 100644 --- a/src/Lucene.Net/Util/IOUtils.cs +++ b/src/Lucene.Net/Util/IOUtils.cs @@ -1,10 +1,13 @@ using J2N; using Lucene.Net.Support; +using Lucene.Net.Support.IO; using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; +using System.Runtime.InteropServices; using System.Text; namespace Lucene.Net.Util @@ -516,8 +519,53 @@ public static void ReThrowUnchecked(Exception th) } } - // LUCENENET specific: Fsync is pointless in .NET, since we are - // calling FileStream.Flush(true) before the stream is disposed - // which means we never need it at the point in Java where it is called. + public static void Fsync(FileSystemInfo fileToSync, bool isDir) + { + // LUCENENET NOTE: there is a bug in 4.8 where it tries to fsync a directory on Windows, + // which is not supported in OpenJDK. This change adopts the latest Lucene code as of 9.10 + // and only fsyncs directories on Linux and macOS. + + // If the file is a directory we have to open read-only, for regular files we must open r/w for + // the fsync to have an effect. + // See http://blog.httrack.com/blog/2013/11/15/everything-you-always-wanted-to-know-about-fsync/ + if (isDir && Constants.WINDOWS) + { + // opening a directory on Windows fails, directories can not be fsynced there + if (fileToSync.Exists == false) + { + // yet do not suppress trying to fsync directories that do not exist + throw new DirectoryNotFoundException(fileToSync.ToString()); + } + return; + } + + try + { + // LUCENENET specific: was: file.force(true); + // We must call fsync on the parent directory, requiring some custom P/Invoking + if (Constants.WINDOWS) + { + WindowsFsyncSupport.Fsync(fileToSync.FullName, isDir); + } + else + { + PosixFsyncSupport.Fsync(fileToSync.FullName, isDir); + } + } + catch (Exception e) when (e.IsIOException()) + { + if (isDir) + { + Debug.Assert((Constants.LINUX || Constants.MAC_OS_X) == false, + "On Linux and MacOSX fsyncing a directory should not throw IOException, " + + "we just don't want to rely on that in production (undocumented). Got: " + + e); + // Ignore exception if it is a directory + return; + } + // Throw original exception + throw; + } + } } } From 9740c22663fc654514c91eba02f35d5b79bb3e1a Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Sat, 11 May 2024 15:23:35 -0600 Subject: [PATCH 02/17] Use System.IO.Directory.Exists to avoid caching exists status --- src/Lucene.Net/Store/FSDirectory.cs | 2 +- src/Lucene.Net/Util/IOUtils.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Lucene.Net/Store/FSDirectory.cs b/src/Lucene.Net/Store/FSDirectory.cs index 746c0309c6..de51d5b9f2 100644 --- a/src/Lucene.Net/Store/FSDirectory.cs +++ b/src/Lucene.Net/Store/FSDirectory.cs @@ -378,7 +378,7 @@ public override void Sync(ICollection names) Fsync(name); } - // fsync the directory itsself, but only if there was any file fsynced before + // fsync the directory itself, but only if there was any file fsynced before // (otherwise it can happen that the directory does not yet exist)! if (toSync.Count > 0) { diff --git a/src/Lucene.Net/Util/IOUtils.cs b/src/Lucene.Net/Util/IOUtils.cs index 74f58c396d..14ee077ff0 100644 --- a/src/Lucene.Net/Util/IOUtils.cs +++ b/src/Lucene.Net/Util/IOUtils.cs @@ -531,10 +531,10 @@ public static void Fsync(FileSystemInfo fileToSync, bool isDir) if (isDir && Constants.WINDOWS) { // opening a directory on Windows fails, directories can not be fsynced there - if (fileToSync.Exists == false) + if (System.IO.Directory.Exists(fileToSync.FullName) == false) { // yet do not suppress trying to fsync directories that do not exist - throw new DirectoryNotFoundException(fileToSync.ToString()); + throw new DirectoryNotFoundException($"Directory does not exist: {fileToSync}"); } return; } From 4c489e2cc83347dd9c4dc030c5d5193c527fe422 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Sat, 11 May 2024 15:38:13 -0600 Subject: [PATCH 03/17] Add unit test for ConcurrentHashSet.ExceptWith --- .../Support/TestConcurrentHashSet.cs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs diff --git a/src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs b/src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs new file mode 100644 index 0000000000..39728c3428 --- /dev/null +++ b/src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs @@ -0,0 +1,51 @@ +using Lucene.Net.Attributes; +using Lucene.Net.Support; +using NUnit.Framework; +using System.Linq; +using System.Threading.Tasks; + +namespace Lucene.Net +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + public class TestConcurrentHashSet + { + [Test, LuceneNetSpecific] + public void TestExceptWith() + { + // Numbers 0-8, 10-80, 99 + var initialSet = Enumerable.Range(1, 8) + .Concat(Enumerable.Range(1, 8).Select(i => i * 10)) + .Append(99) + .Append(0); + + var hashSet = new ConcurrentHashSet(initialSet); + + Parallel.ForEach(Enumerable.Range(1, 8), i => + { + // Remove i and i * 10, i.e. 1 and 10, 2 and 20, etc. + var except = new[] { i, i * 10 }; + hashSet.ExceptWith(except); + }); + + Assert.AreEqual(2, hashSet.Count); + Assert.IsTrue(hashSet.Contains(0)); + Assert.IsTrue(hashSet.Contains(99)); + } + } +} From c57f6b3be8a0a14609e9592a77a59729a999ca96 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Sat, 11 May 2024 16:52:09 -0600 Subject: [PATCH 04/17] Improve errors thrown by CreateFileW --- src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs b/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs index 09666b88a7..347f4e0f64 100644 --- a/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs +++ b/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs @@ -45,6 +45,9 @@ IntPtr hTemplateFile private static readonly IntPtr INVALID_HANDLE_VALUE = new IntPtr(-1); + // https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499- + private const int ERROR_FILE_NOT_FOUND = 2; + private const int ERROR_PATH_NOT_FOUND = 3; private const int ERROR_ACCESS_DENIED = 5; // https://learn.microsoft.com/en-us/windows/win32/secauthz/generic-access-rights @@ -79,7 +82,13 @@ public HandleWrapper(string path, bool isDir) if (handle == INVALID_HANDLE_VALUE) { int error = Marshal.GetLastWin32Error(); - throw new IOException($"Unable to open directory, error: 0x{error:x8}", error); + + throw error switch { + ERROR_FILE_NOT_FOUND => new FileNotFoundException($"File not found: {path}"), + ERROR_PATH_NOT_FOUND => new DirectoryNotFoundException($"Directory/path not found: {path}"), + ERROR_ACCESS_DENIED => new UnauthorizedAccessException($"Access denied to {(isDir ? "directory" : "file")}: {path}"), + _ => new IOException($"Unable to open {(isDir ? "directory" : "file")}, error: 0x{error:x8}", error) + }; } } From 50f7b0666c84525f75c98e05a548d00c690cb171 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Tue, 14 May 2024 20:13:41 -0600 Subject: [PATCH 05/17] Change FileSystemInfo use to string in IOUtils.Fsync --- src/Lucene.Net/Store/FSDirectory.cs | 4 ++-- src/Lucene.Net/Util/IOUtils.cs | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Lucene.Net/Store/FSDirectory.cs b/src/Lucene.Net/Store/FSDirectory.cs index de51d5b9f2..3eaa3f123d 100644 --- a/src/Lucene.Net/Store/FSDirectory.cs +++ b/src/Lucene.Net/Store/FSDirectory.cs @@ -382,7 +382,7 @@ public override void Sync(ICollection names) // (otherwise it can happen that the directory does not yet exist)! if (toSync.Count > 0) { - IOUtils.Fsync(m_directory, true); + IOUtils.Fsync(m_directory.FullName, true); } m_staleFiles.ExceptWith(toSync); @@ -579,7 +579,7 @@ public override void Seek(long pos) protected virtual void Fsync(string name) { - IOUtils.Fsync(new FileInfo(Path.Combine(m_directory.FullName, name)), false); + IOUtils.Fsync(Path.Combine(m_directory.FullName, name), false); } } } diff --git a/src/Lucene.Net/Util/IOUtils.cs b/src/Lucene.Net/Util/IOUtils.cs index 14ee077ff0..eb635573f7 100644 --- a/src/Lucene.Net/Util/IOUtils.cs +++ b/src/Lucene.Net/Util/IOUtils.cs @@ -519,7 +519,8 @@ public static void ReThrowUnchecked(Exception th) } } - public static void Fsync(FileSystemInfo fileToSync, bool isDir) + // LUCENENET specific: using string instead of FileSystemInfo to avoid extra allocation + public static void Fsync(string fileToSync, bool isDir) { // LUCENENET NOTE: there is a bug in 4.8 where it tries to fsync a directory on Windows, // which is not supported in OpenJDK. This change adopts the latest Lucene code as of 9.10 @@ -531,7 +532,7 @@ public static void Fsync(FileSystemInfo fileToSync, bool isDir) if (isDir && Constants.WINDOWS) { // opening a directory on Windows fails, directories can not be fsynced there - if (System.IO.Directory.Exists(fileToSync.FullName) == false) + if (System.IO.Directory.Exists(fileToSync) == false) { // yet do not suppress trying to fsync directories that do not exist throw new DirectoryNotFoundException($"Directory does not exist: {fileToSync}"); @@ -545,11 +546,11 @@ public static void Fsync(FileSystemInfo fileToSync, bool isDir) // We must call fsync on the parent directory, requiring some custom P/Invoking if (Constants.WINDOWS) { - WindowsFsyncSupport.Fsync(fileToSync.FullName, isDir); + WindowsFsyncSupport.Fsync(fileToSync, isDir); } else { - PosixFsyncSupport.Fsync(fileToSync.FullName, isDir); + PosixFsyncSupport.Fsync(fileToSync, isDir); } } catch (Exception e) when (e.IsIOException()) From 5f59ce17c6c96a332688358bb5f938185158c6db Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Tue, 14 May 2024 20:16:54 -0600 Subject: [PATCH 06/17] Change Debug.Assert to Debugging use --- src/Lucene.Net/Util/IOUtils.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Lucene.Net/Util/IOUtils.cs b/src/Lucene.Net/Util/IOUtils.cs index eb635573f7..6e31584a63 100644 --- a/src/Lucene.Net/Util/IOUtils.cs +++ b/src/Lucene.Net/Util/IOUtils.cs @@ -1,4 +1,5 @@ using J2N; +using Lucene.Net.Diagnostics; using Lucene.Net.Support; using Lucene.Net.Support.IO; using System; @@ -557,10 +558,13 @@ public static void Fsync(string fileToSync, bool isDir) { if (isDir) { - Debug.Assert((Constants.LINUX || Constants.MAC_OS_X) == false, - "On Linux and MacOSX fsyncing a directory should not throw IOException, " - + "we just don't want to rely on that in production (undocumented). Got: " - + e); + if (Debugging.AssertsEnabled) + { + Debugging.Assert((Constants.LINUX || Constants.MAC_OS_X) == false, + "On Linux and MacOSX fsyncing a directory should not throw IOException, we just don't want to rely on that in production (undocumented). Got: {0}", + e); + } + // Ignore exception if it is a directory return; } From 3635caac4684c21e30210a2602928ad429ff347e Mon Sep 17 00:00:00 2001 From: Shad Storhaug Date: Wed, 15 May 2024 09:22:37 +0700 Subject: [PATCH 07/17] Lucene.Net.Index.TestIndexWriterOnJRECrash::TestNRTThreads_Mem(): Removed AwaitsFix attribute. The FSync implementation should fix this test. --- src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs b/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs index 32e1f4c765..229c686338 100644 --- a/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs +++ b/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 From 268295259beb9e163bdd304aaff43924e312d0b2 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Tue, 14 May 2024 22:20:45 -0600 Subject: [PATCH 08/17] Make ExceptWith atomic --- src/Lucene.Net/Support/ConcurrentHashSet.cs | 74 +++++++++++++++++---- 1 file changed, 61 insertions(+), 13 deletions(-) diff --git a/src/Lucene.Net/Support/ConcurrentHashSet.cs b/src/Lucene.Net/Support/ConcurrentHashSet.cs index 917b3ce356..877e4685e0 100644 --- a/src/Lucene.Net/Support/ConcurrentHashSet.cs +++ b/src/Lucene.Net/Support/ConcurrentHashSet.cs @@ -74,9 +74,9 @@ public int Count { AcquireAllLocks(ref acquiredLocks); - for (var i = 0; i < _tables.CountPerLock.Length; i++) + foreach (var t in _tables.CountPerLock) { - count += _tables.CountPerLock[i]; + count += t; } } finally @@ -88,6 +88,21 @@ public int Count } } + private int CountInternal + { + get + { + int count = 0; + + foreach (var t in _tables.CountPerLock) + { + count += t; + } + + return count; + } + } + /// /// Gets a value that indicates whether the is empty. /// @@ -298,9 +313,7 @@ public void Clear() { AcquireAllLocks(ref locksAcquired); - var newTables = new Tables(new Node[DefaultCapacity], _tables.Locks, new int[_tables.CountPerLock.Length]); - _tables = newTables; - _budget = Math.Max(1, newTables.Buckets.Length / newTables.Locks.Length); + ClearInternal(); } finally { @@ -308,6 +321,13 @@ public void Clear() } } + private void ClearInternal() + { + var newTables = new Tables(new Node[DefaultCapacity], _tables.Locks, new int[_tables.CountPerLock.Length]); + _tables = newTables; + _budget = Math.Max(1, newTables.Buckets.Length / newTables.Locks.Length); + } + /// /// Determines whether the contains the specified /// item. @@ -347,6 +367,11 @@ public bool Contains(T item) public bool TryRemove(T item) { var hashcode = _comparer.GetHashCode(item); + return TryRemoveInternal(item, hashcode, acquireLock: true); + } + + private bool TryRemoveInternal(T item, int hashcode, bool acquireLock) + { while (true) { var tables = _tables; @@ -354,9 +379,13 @@ public bool TryRemove(T item) GetBucketAndLockNo(hashcode, out int bucketNo, out int lockNo, tables.Buckets.Length, tables.Locks.Length); object syncRoot = tables.Locks[lockNo]; - UninterruptableMonitor.Enter(syncRoot); + var lockTaken = false; + try { + if (acquireLock) + UninterruptableMonitor.Enter(syncRoot, ref lockTaken); + // If the table just got resized, we may not be holding the right lock, and must retry. // This should be a rare occurrence. if (tables != _tables) @@ -388,7 +417,8 @@ public bool TryRemove(T item) } finally { - UninterruptableMonitor.Exit(syncRoot); + if (lockTaken) + UninterruptableMonitor.Exit(syncRoot); } return false; @@ -753,15 +783,33 @@ private void CopyToItems(T[] array, int index) public void ExceptWith(IEnumerable other) { - if (ReferenceEquals(this, other)) + if (other is null) + throw new ArgumentNullException(nameof(other)); + + var locksAcquired = 0; + try { - Clear(); - return; - } + AcquireAllLocks(ref locksAcquired); + + if (CountInternal == 0) + { + return; + } + + if (ReferenceEquals(this, other)) + { + ClearInternal(); + return; + } - foreach (var item in other) + foreach (var item in other) + { + TryRemoveInternal(item, _comparer.GetHashCode(item), acquireLock: false); + } + } + finally { - TryRemove(item); + ReleaseLocks(0, locksAcquired); } } From 21c7196ab9a72e8452f2154b3c6d6d27d5ed840e Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Tue, 14 May 2024 22:45:52 -0600 Subject: [PATCH 09/17] Improve error handling if directory not found on Linux/macOS --- .../Support/IO/PosixFsyncSupport.cs | 22 +++++++++++++---- .../Support/IO/WindowsFsyncSupport.cs | 3 ++- src/Lucene.Net/Util/IOUtils.cs | 24 +++++++++---------- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs b/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs index e4d6c833a0..647f44998f 100644 --- a/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs +++ b/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs @@ -1,4 +1,5 @@ using Lucene.Net.Util; +using System; using System.IO; using System.Runtime.InteropServices; @@ -46,6 +47,9 @@ internal static class PosixFsyncSupport // https://opensource.apple.com/source/xnu/xnu-6153.81.5/bsd/sys/fcntl.h.auto.html private const int F_FULLFSYNC = 51; + private const int EACCES = 13; + private const int ENOENT = 2; + public static void Fsync(string path, bool isDir) { using DescriptorWrapper handle = new DescriptorWrapper(path, isDir); @@ -63,7 +67,14 @@ public DescriptorWrapper(string path, bool isDir) if (fd == -1) { int error = Marshal.GetLastWin32Error(); - throw new IOException($"Unable to open path, error: 0x{error:x8}", error); + + throw error switch + { + ENOENT when isDir => new DirectoryNotFoundException($"Directory/path not found: {path}"), + ENOENT => new FileNotFoundException($"File not found: {path}"), + EACCES => new UnauthorizedAccessException($"Access denied to {(isDir ? "directory" : "file")}: {path}"), + _ => new IOException($"Unable to open path, error: 0x{error:x8}", error) + }; } } @@ -74,12 +85,14 @@ public void Flush() { if (fcntl(fd, F_FULLFSYNC, 0) == -1) { - throw new IOException("fcntl failed", Marshal.GetLastWin32Error()); + int error = Marshal.GetLastWin32Error(); + throw new IOException($"fcntl failed, error: 0x{error:x8}", error); } } else if (fsync(fd) == -1) { - throw new IOException("fsync failed", Marshal.GetLastWin32Error()); + int error = Marshal.GetLastWin32Error(); + throw new IOException($"fsync failed, error: 0x{error:x8}", error); } } @@ -87,7 +100,8 @@ public void Dispose() { if (close(fd) == -1) { - throw new IOException("close failed", Marshal.GetLastWin32Error()); + int error = Marshal.GetLastWin32Error(); + throw new IOException($"close failed, error: 0x{error:x8}", error); } } } diff --git a/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs b/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs index 347f4e0f64..d9ab14f256 100644 --- a/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs +++ b/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs @@ -83,7 +83,8 @@ public HandleWrapper(string path, bool isDir) { int error = Marshal.GetLastWin32Error(); - throw error switch { + throw error switch + { ERROR_FILE_NOT_FOUND => new FileNotFoundException($"File not found: {path}"), ERROR_PATH_NOT_FOUND => new DirectoryNotFoundException($"Directory/path not found: {path}"), ERROR_ACCESS_DENIED => new UnauthorizedAccessException($"Access denied to {(isDir ? "directory" : "file")}: {path}"), diff --git a/src/Lucene.Net/Util/IOUtils.cs b/src/Lucene.Net/Util/IOUtils.cs index 6e31584a63..dc17cdbdd9 100644 --- a/src/Lucene.Net/Util/IOUtils.cs +++ b/src/Lucene.Net/Util/IOUtils.cs @@ -554,22 +554,20 @@ public static void Fsync(string fileToSync, bool isDir) PosixFsyncSupport.Fsync(fileToSync, isDir); } } - catch (Exception e) when (e.IsIOException()) + catch (Exception e) when (e.IsIOException() && isDir && e is not DirectoryNotFoundException) { - if (isDir) - { - if (Debugging.AssertsEnabled) - { - Debugging.Assert((Constants.LINUX || Constants.MAC_OS_X) == false, - "On Linux and MacOSX fsyncing a directory should not throw IOException, we just don't want to rely on that in production (undocumented). Got: {0}", - e); - } + // LUCENENET specific - make catch specific to IOExceptions when it's a directory, + // but allow DirectoryNotFoundException to pass through as an equivalent would normally be + // thrown by the FileChannel.open call in Java which is outside the try block. - // Ignore exception if it is a directory - return; + if (Debugging.AssertsEnabled) + { + Debugging.Assert((Constants.LINUX || Constants.MAC_OS_X) == false, + "On Linux and MacOSX fsyncing a directory should not throw IOException, we just don't want to rely on that in production (undocumented). Got: {0}", + e); } - // Throw original exception - throw; + + // Ignore exception if it is a directory } } } From 2623a01f105f5b4fc85aa4ad2e9291f19b42875c Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Tue, 14 May 2024 23:05:20 -0600 Subject: [PATCH 10/17] Refactor interop methods into separate partial class files --- .../Support/IO/PosixFsyncSupport.cs | 28 +----------- .../Support/IO/WindowsFsyncSupport.cs | 37 +--------------- .../Support/Native/Interop.MacOS.Constants.cs | 28 ++++++++++++ .../Support/Native/Interop.Posix.Close.cs | 31 +++++++++++++ .../Support/Native/Interop.Posix.Constants.cs | 31 +++++++++++++ .../Support/Native/Interop.Posix.Fcntl.cs | 32 ++++++++++++++ .../Support/Native/Interop.Posix.Fsync.cs | 31 +++++++++++++ .../Support/Native/Interop.Posix.Open.cs | 31 +++++++++++++ .../Native/Interop.Win32.CloseHandle.cs | 32 ++++++++++++++ .../Support/Native/Interop.Win32.Constants.cs | 43 +++++++++++++++++++ .../Native/Interop.Win32.CreateFileW.cs | 40 +++++++++++++++++ .../Native/Interop.Win32.FlushFileBuffers.cs | 32 ++++++++++++++ 12 files changed, 334 insertions(+), 62 deletions(-) create mode 100644 src/Lucene.Net/Support/Native/Interop.MacOS.Constants.cs create mode 100644 src/Lucene.Net/Support/Native/Interop.Posix.Close.cs create mode 100644 src/Lucene.Net/Support/Native/Interop.Posix.Constants.cs create mode 100644 src/Lucene.Net/Support/Native/Interop.Posix.Fcntl.cs create mode 100644 src/Lucene.Net/Support/Native/Interop.Posix.Fsync.cs create mode 100644 src/Lucene.Net/Support/Native/Interop.Posix.Open.cs create mode 100644 src/Lucene.Net/Support/Native/Interop.Win32.CloseHandle.cs create mode 100644 src/Lucene.Net/Support/Native/Interop.Win32.Constants.cs create mode 100644 src/Lucene.Net/Support/Native/Interop.Win32.CreateFileW.cs create mode 100644 src/Lucene.Net/Support/Native/Interop.Win32.FlushFileBuffers.cs diff --git a/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs b/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs index 647f44998f..352739ff49 100644 --- a/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs +++ b/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs @@ -2,6 +2,8 @@ using System; using System.IO; using System.Runtime.InteropServices; +using static Lucene.Net.Support.Native.Interop.Posix; +using static Lucene.Net.Support.Native.Interop.MacOS; namespace Lucene.Net.Support.IO { @@ -24,32 +26,6 @@ namespace Lucene.Net.Support.IO internal static class PosixFsyncSupport { - // https://pubs.opengroup.org/onlinepubs/009695399/functions/fsync.html - [DllImport("libc", SetLastError = true)] - private static extern int fsync(int fd); - - // https://pubs.opengroup.org/onlinepubs/007904875/functions/open.html - [DllImport("libc", SetLastError = true)] - private static extern int open([MarshalAs(UnmanagedType.LPStr)] string pathname, int flags); - - // https://pubs.opengroup.org/onlinepubs/009604499/functions/close.html - [DllImport("libc", SetLastError = true)] - private static extern int close(int fd); - - // https://pubs.opengroup.org/onlinepubs/007904975/functions/fcntl.html - // and https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fcntl.2.html - [DllImport("libc", SetLastError = true)] - private static extern int fcntl(int fd, int cmd, int arg); - - private const int O_RDONLY = 0; - private const int O_WRONLY = 1; - - // https://opensource.apple.com/source/xnu/xnu-6153.81.5/bsd/sys/fcntl.h.auto.html - private const int F_FULLFSYNC = 51; - - private const int EACCES = 13; - private const int ENOENT = 2; - public static void Fsync(string path, bool isDir) { using DescriptorWrapper handle = new DescriptorWrapper(path, isDir); diff --git a/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs b/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs index d9ab14f256..78017a20fa 100644 --- a/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs +++ b/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Runtime.InteropServices; +using static Lucene.Net.Support.Native.Interop.Win32; namespace Lucene.Net.Support.IO { @@ -23,42 +24,6 @@ namespace Lucene.Net.Support.IO public static class WindowsFsyncSupport { - // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew - [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] - private static extern IntPtr CreateFileW( - string lpFileName, - uint dwDesiredAccess, - uint dwShareMode, - IntPtr lpSecurityAttributes, - uint dwCreationDisposition, - uint dwFlagsAndAttributes, - IntPtr hTemplateFile - ); - - // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-flushfilebuffers - [DllImport("kernel32.dll", SetLastError = true)] - private static extern bool FlushFileBuffers(IntPtr hFile); - - // https://learn.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-closehandle - [DllImport("kernel32.dll", SetLastError = true)] - private static extern bool CloseHandle(IntPtr hObject); - - private static readonly IntPtr INVALID_HANDLE_VALUE = new IntPtr(-1); - - // https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499- - private const int ERROR_FILE_NOT_FOUND = 2; - private const int ERROR_PATH_NOT_FOUND = 3; - private const int ERROR_ACCESS_DENIED = 5; - - // https://learn.microsoft.com/en-us/windows/win32/secauthz/generic-access-rights - private const int GENERIC_WRITE = 0x40000000; - - private const int FILE_SHARE_READ = 0x00000001; - private const int FILE_SHARE_WRITE = 0x00000002; - private const int FILE_SHARE_DELETE = 0x00000004; - private const int FILE_FLAG_BACKUP_SEMANTICS = 0x02000000; - private const int OPEN_EXISTING = 3; - public static void Fsync(string path, bool isDir) { using HandleWrapper handle = new HandleWrapper(path, isDir); diff --git a/src/Lucene.Net/Support/Native/Interop.MacOS.Constants.cs b/src/Lucene.Net/Support/Native/Interop.MacOS.Constants.cs new file mode 100644 index 0000000000..b0ba4613d8 --- /dev/null +++ b/src/Lucene.Net/Support/Native/Interop.MacOS.Constants.cs @@ -0,0 +1,28 @@ +namespace Lucene.Net.Support.Native +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + internal static partial class Interop + { + internal static partial class MacOS + { + // https://opensource.apple.com/source/xnu/xnu-6153.81.5/bsd/sys/fcntl.h.auto.html + internal const int F_FULLFSYNC = 51; + } + } +} diff --git a/src/Lucene.Net/Support/Native/Interop.Posix.Close.cs b/src/Lucene.Net/Support/Native/Interop.Posix.Close.cs new file mode 100644 index 0000000000..009b0daf45 --- /dev/null +++ b/src/Lucene.Net/Support/Native/Interop.Posix.Close.cs @@ -0,0 +1,31 @@ +using System.Runtime.InteropServices; + +namespace Lucene.Net.Support.Native +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + internal static partial class Interop + { + internal static partial class Posix + { + // https://pubs.opengroup.org/onlinepubs/009604499/functions/close.html + [DllImport("libc", SetLastError = true)] + internal static extern int close(int fd); + } + } +} diff --git a/src/Lucene.Net/Support/Native/Interop.Posix.Constants.cs b/src/Lucene.Net/Support/Native/Interop.Posix.Constants.cs new file mode 100644 index 0000000000..d483a7d9ff --- /dev/null +++ b/src/Lucene.Net/Support/Native/Interop.Posix.Constants.cs @@ -0,0 +1,31 @@ +namespace Lucene.Net.Support.Native +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + internal static partial class Interop + { + internal static partial class Posix + { + internal const int O_RDONLY = 0; + internal const int O_WRONLY = 1; + + internal const int EACCES = 13; + internal const int ENOENT = 2; + } + } +} diff --git a/src/Lucene.Net/Support/Native/Interop.Posix.Fcntl.cs b/src/Lucene.Net/Support/Native/Interop.Posix.Fcntl.cs new file mode 100644 index 0000000000..42a6eb9135 --- /dev/null +++ b/src/Lucene.Net/Support/Native/Interop.Posix.Fcntl.cs @@ -0,0 +1,32 @@ +using System.Runtime.InteropServices; + +namespace Lucene.Net.Support.Native +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + internal static partial class Interop + { + internal static partial class Posix + { + // https://pubs.opengroup.org/onlinepubs/007904975/functions/fcntl.html + // and https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fcntl.2.html + [DllImport("libc", SetLastError = true)] + internal static extern int fcntl(int fd, int cmd, int arg); + } + } +} diff --git a/src/Lucene.Net/Support/Native/Interop.Posix.Fsync.cs b/src/Lucene.Net/Support/Native/Interop.Posix.Fsync.cs new file mode 100644 index 0000000000..f63116596d --- /dev/null +++ b/src/Lucene.Net/Support/Native/Interop.Posix.Fsync.cs @@ -0,0 +1,31 @@ +using System.Runtime.InteropServices; + +namespace Lucene.Net.Support.Native +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + internal static partial class Interop + { + internal static partial class Posix + { + // https://pubs.opengroup.org/onlinepubs/009695399/functions/fsync.html + [DllImport("libc", SetLastError = true)] + internal static extern int fsync(int fd); + } + } +} diff --git a/src/Lucene.Net/Support/Native/Interop.Posix.Open.cs b/src/Lucene.Net/Support/Native/Interop.Posix.Open.cs new file mode 100644 index 0000000000..0ef5b9c49e --- /dev/null +++ b/src/Lucene.Net/Support/Native/Interop.Posix.Open.cs @@ -0,0 +1,31 @@ +using System.Runtime.InteropServices; + +namespace Lucene.Net.Support.Native +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + internal static partial class Interop + { + internal static partial class Posix + { + // https://pubs.opengroup.org/onlinepubs/007904875/functions/open.html + [DllImport("libc", SetLastError = true)] + internal static extern int open([MarshalAs(UnmanagedType.LPStr)] string pathname, int flags); + } + } +} diff --git a/src/Lucene.Net/Support/Native/Interop.Win32.CloseHandle.cs b/src/Lucene.Net/Support/Native/Interop.Win32.CloseHandle.cs new file mode 100644 index 0000000000..9c8f56fda7 --- /dev/null +++ b/src/Lucene.Net/Support/Native/Interop.Win32.CloseHandle.cs @@ -0,0 +1,32 @@ +using System; +using System.Runtime.InteropServices; + +namespace Lucene.Net.Support.Native +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + internal static partial class Interop + { + internal static partial class Win32 + { + // https://learn.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-closehandle + [DllImport("kernel32.dll", SetLastError = true)] + internal static extern bool CloseHandle(IntPtr hObject); + } + } +} diff --git a/src/Lucene.Net/Support/Native/Interop.Win32.Constants.cs b/src/Lucene.Net/Support/Native/Interop.Win32.Constants.cs new file mode 100644 index 0000000000..313dec7e19 --- /dev/null +++ b/src/Lucene.Net/Support/Native/Interop.Win32.Constants.cs @@ -0,0 +1,43 @@ +using System; + +namespace Lucene.Net.Support.Native +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + internal static partial class Interop + { + internal static partial class Win32 + { + internal static readonly IntPtr INVALID_HANDLE_VALUE = new IntPtr(-1); + + // https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499- + internal const int ERROR_FILE_NOT_FOUND = 2; + internal const int ERROR_PATH_NOT_FOUND = 3; + internal const int ERROR_ACCESS_DENIED = 5; + + // https://learn.microsoft.com/en-us/windows/win32/secauthz/generic-access-rights + internal const int GENERIC_WRITE = 0x40000000; + + internal const int FILE_SHARE_READ = 0x00000001; + internal const int FILE_SHARE_WRITE = 0x00000002; + internal const int FILE_SHARE_DELETE = 0x00000004; + internal const int FILE_FLAG_BACKUP_SEMANTICS = 0x02000000; + internal const int OPEN_EXISTING = 3; + } + } +} diff --git a/src/Lucene.Net/Support/Native/Interop.Win32.CreateFileW.cs b/src/Lucene.Net/Support/Native/Interop.Win32.CreateFileW.cs new file mode 100644 index 0000000000..8c52113dc9 --- /dev/null +++ b/src/Lucene.Net/Support/Native/Interop.Win32.CreateFileW.cs @@ -0,0 +1,40 @@ +using System; +using System.Runtime.InteropServices; + +namespace Lucene.Net.Support.Native +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + internal static partial class Interop + { + internal static partial class Win32 + { + // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + internal static extern IntPtr CreateFileW( + string lpFileName, + uint dwDesiredAccess, + uint dwShareMode, + IntPtr lpSecurityAttributes, + uint dwCreationDisposition, + uint dwFlagsAndAttributes, + IntPtr hTemplateFile + ); + } + } +} diff --git a/src/Lucene.Net/Support/Native/Interop.Win32.FlushFileBuffers.cs b/src/Lucene.Net/Support/Native/Interop.Win32.FlushFileBuffers.cs new file mode 100644 index 0000000000..ad448a9795 --- /dev/null +++ b/src/Lucene.Net/Support/Native/Interop.Win32.FlushFileBuffers.cs @@ -0,0 +1,32 @@ +using System; +using System.Runtime.InteropServices; + +namespace Lucene.Net.Support.Native +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + internal static partial class Interop + { + internal static partial class Win32 + { + // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-flushfilebuffers + [DllImport("kernel32.dll", SetLastError = true)] + internal static extern bool FlushFileBuffers(IntPtr hFile); + } + } +} From f3f3459b376df061ca3884850b392b8fc17a79ea Mon Sep 17 00:00:00 2001 From: Shad Storhaug Date: Wed, 15 May 2024 10:30:15 +0700 Subject: [PATCH 11/17] Lucene.Net.Index.TestIndexWriterOnJRECrash::TestNRTThreads_Mem(): Added [Repeat(25)] attribute. --- src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs b/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs index 229c686338..162cd429d0 100644 --- a/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs +++ b/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs @@ -47,6 +47,7 @@ public class TestIndexWriterOnJRECrash : TestNRTThreads [Test] [Slow] + [Repeat(25)] public override void TestNRTThreads_Mem() { //if we are not the fork From 72649b2dafb4a06af78d5336d7e8fbdf3ba4ca09 Mon Sep 17 00:00:00 2001 From: Shad Storhaug Date: Wed, 15 May 2024 13:44:16 +0700 Subject: [PATCH 12/17] Lucene.Net.Index.TestIndexWriterOnJRECrash: Instead of using a temp file to pass the process ID to kill back to the original test process, open a socket and listen for the process ID to be written. --- .../Index/TestIndexWriterOnJRECrash.cs | 109 ++++++++++-------- 1 file changed, 64 insertions(+), 45 deletions(-) diff --git a/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs b/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs index 162cd429d0..79b6ca15e0 100644 --- a/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs +++ b/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs @@ -4,13 +4,13 @@ using NUnit.Framework; using RandomizedTesting.Generators; using System; -using System.Data; using System.Diagnostics; using System.Globalization; using System.IO; using System.Linq; +using System.Net; +using System.Net.Sockets; using System.Reflection; -using System.Text; using System.Threading; using BaseDirectoryWrapper = Lucene.Net.Store.BaseDirectoryWrapper; using Console = Lucene.Net.Util.SystemConsole; @@ -51,7 +51,7 @@ public class TestIndexWriterOnJRECrash : TestNRTThreads public override void TestNRTThreads_Mem() { //if we are not the fork - if (SystemProperties.GetProperty("tests:crashmode") is null) + if (!SystemProperties.GetPropertyAsBoolean("tests:crashmode", false)) { // try up to 10 times to create an index for (int i = 0; i < 10; i++) @@ -63,18 +63,18 @@ public override void TestNRTThreads_Mem() // lexicographical order rather than checking the one we create in the current iteration. DirectoryInfo tempDir = CreateTempDir("netcrash"); - FileInfo tempProcessToKillFile = CreateTempFile(prefix: "netcrash-processToKill", suffix: ".txt"); - tempProcessToKillFile.Delete(); // We use the creation of this file as a signal to parse it. + // Set up a TCP listener to receive the process ID + int port = SetupSocketListener(out TcpListener listener); // Note this is the vstest.console process we are tracking here. - Process p = ForkTest(tempDir.FullName, tempProcessToKillFile.FullName); + Process p = ForkTest(tempDir.FullName, port); TextWriter childOut = BeginOutput(p, out ThreadJob stdOutPumper, out ThreadJob stdErrPumper); // LUCENENET: Note that ForkTest() creates the vstest.console.exe process. // This spawns testhost.exe, which runs our test. We wait until - // the process starts and logs its own Id so we know who to kill later. - int processIdToKill = WaitForProcessToKillLogFile(tempProcessToKillFile.FullName); + // the process starts and transmits its own Id so we know who to kill later. + int processIdToKill = WaitForProcessId(listener); // Setup a time to crash the forked thread int crashTime = TestUtil.NextInt32(Random, 4000, 5000); // LUCENENET: Adjusted these up by 1 second to give our tests some more time to spin up @@ -100,13 +100,12 @@ public override void TestNRTThreads_Mem() // we are the fork, log our processId so the original test can kill us. int processIdToKill = Process.GetCurrentProcess().Id; - string processIdToKillFile = SystemProperties.GetProperty("tests:tempProcessToKillFile"); + int port = SystemProperties.GetPropertyAsInt32("tests:crashtestport", -1); - assertNotNull("No tests:tempProcessToKillFile value was passed to the fork. This is a required system property.", processIdToKillFile); + assertTrue("No tests:crashtestport value was passed to the fork. This is a required system property.", port > 0); - // Writing this file will kick off the thread that crashes us. - using (var writer = new StreamWriter(processIdToKillFile, append: false, Encoding.UTF8, bufferSize: 32)) - writer.WriteLine(processIdToKill.ToString(CultureInfo.InvariantCulture)); + // Sending the process id will kick off the thread that crashes us. + SendProcessId(processIdToKill, port); // run the test until we crash. for (int i = 0; i < 100; i++) @@ -145,7 +144,7 @@ public override void Run() } } - public Process ForkTest(string tempDir, string tempProcessToKillFile) + public Process ForkTest(string tempDir, int port) { //get the full location of the assembly with DaoTests in it string testAssemblyPath = Assembly.GetAssembly(typeof(TestIndexWriterOnJRECrash)).Location; @@ -174,8 +173,8 @@ public Process ForkTest(string tempDir, string tempProcessToKillFile) // passing NIGHTLY to this test makes it run for much longer, easier to catch it in the act... TestRunParameter("tests:nightly", "true"), TestRunParameter("tempDir", tempDir), - // This file is for passing the process ID of the fork back to the original test so it can kill it. - TestRunParameter("tests:tempProcessToKillFile", tempProcessToKillFile), + // This port is for passing the process ID of the fork back to the original test so it can kill it. + TestRunParameter("tests:crashtestport", port.ToString(CultureInfo.InvariantCulture)), }), WorkingDirectory = theDirectory, RedirectStandardOutput = true, @@ -335,25 +334,38 @@ public virtual bool CheckIndexes(FileSystemInfo file) return false; } - // LUCENENET: Wait for our test to spin up and log its PID so we can kill it. - private static int WaitForProcessToKillLogFile(string processToKillFile) + private int SetupSocketListener(out TcpListener listener) { - bool exists = false; - Thread.Sleep(500); - for (int i = 0; i < 150; i++) + // Pick a random port that is available on the local machine. + listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + return ((IPEndPoint)listener.LocalEndpoint).Port; + } + + // LUCENENET: Wait for our test to spin up and send its process ID so we can kill it. + private int WaitForProcessId(TcpListener listener) + { + try { - if (File.Exists(processToKillFile)) - { - exists = true; - break; - } - Thread.Sleep(200); + using var client = listener.AcceptTcpClient(); + using var stream = client.GetStream(); + // Directly read the process ID as a 32-bit integer + using var reader = new BinaryReader(stream); + return reader.ReadInt32(); } - // If the fork didn't log its process id, it is a failure. - assertTrue("The test fork didn't log its process id, so we cannot kill it", exists); - using var reader = new StreamReader(processToKillFile, Encoding.UTF8); - // LUCENENET: Our file only has one line with the process Id in it - return int.Parse(reader.ReadLine().Trim(), CultureInfo.InvariantCulture); + finally + { + listener.Stop(); + } + } + + private void SendProcessId(int processId, int port) + { + using var client = new TcpClient("127.0.0.1", port); + using var stream = client.GetStream(); + // Directly write the process ID as a 32-bit integer + using var writer = new BinaryWriter(stream); + writer.Write(processId); } public virtual void CrashDotNet(int processIdToKill) @@ -361,22 +373,29 @@ public virtual void CrashDotNet(int processIdToKill) Process process = null; try { - process = Process.GetProcessById(processIdToKill); - } - catch (ArgumentException) - { - // We get here if the process wasn't running for some reason. - // We should fix the forked test to make it run longer if we get here. - fail("The test completed before we could kill it."); - } + try + { + process = Process.GetProcessById(processIdToKill); + } + catch (ArgumentException) + { + // We get here if the process wasn't running for some reason. + // We should fix the forked test to make it run longer if we get here. + fail("The test completed before we could kill it."); + } #if FEATURE_PROCESS_KILL_ENTIREPROCESSTREE - process.Kill(entireProcessTree: true); + process.Kill(entireProcessTree: true); #else - process.Kill(); + process.Kill(); #endif - process.WaitForExit(10000); - // We couldn't get .NET to crash for some reason. - assertTrue(process.HasExited); + process.WaitForExit(10000); + // We couldn't get .NET to crash for some reason. + assertTrue(process.HasExited); + } + finally + { + process?.Dispose(); + } } } } From 8d977135c228c8322609ee9209b9b7f235a384bc Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Wed, 15 May 2024 20:19:17 -0600 Subject: [PATCH 13/17] Synchronize access to stale files collection This is necessary to prevent race conditions, even though this code is not in the upstream Java code. A thread could try to add an item to the collection after it has been synced in `Sync` but before it is removed from the collection, then the file is removed from the collection, resulting in a missed sync. --- src/Lucene.Net/Store/FSDirectory.cs | 108 ++++++++++++++++++++-------- 1 file changed, 77 insertions(+), 31 deletions(-) diff --git a/src/Lucene.Net/Store/FSDirectory.cs b/src/Lucene.Net/Store/FSDirectory.cs index 3eaa3f123d..63d92f9cca 100644 --- a/src/Lucene.Net/Store/FSDirectory.cs +++ b/src/Lucene.Net/Store/FSDirectory.cs @@ -1,5 +1,7 @@ using Lucene.Net.Support; using Lucene.Net.Support.IO; +using Lucene.Net.Support.Threading; +using Lucene.Net.Util; using System; using System.Collections.Generic; using System.Globalization; @@ -29,9 +31,6 @@ namespace Lucene.Net.Store * limitations under the License. */ - using Constants = Lucene.Net.Util.Constants; - using IOUtils = Lucene.Net.Util.IOUtils; - /// /// Base class for implementations that store index /// files in the file system. @@ -80,7 +79,7 @@ namespace Lucene.Net.Store /// in .NET /// in conjunction with an open because it is not guaranteed to exit atomically. /// Any lock statement or call can throw a - /// , which makes shutting down unpredictable. + /// , which makes shutting down unpredictable. /// To exit parallel tasks safely, we recommend using s /// and "interrupt" them with s. /// @@ -94,7 +93,24 @@ public abstract class FSDirectory : BaseDirectory public const int DEFAULT_READ_CHUNK_SIZE = 8192; protected readonly DirectoryInfo m_directory; // The underlying filesystem directory - protected readonly ISet m_staleFiles = new ConcurrentHashSet(); // Files written, but not yet sync'ed + + /// + /// The collection of stale files that need to be 'ed + /// + /// + /// LUCENENET NOTE: This is a non-thread-safe collection so that we can synchronize access to it + /// using the field. This is to prevent race conditions, i.e. one thread + /// adding a file to the collection while another thread is trying to sync the files, which could + /// cause a missed sync. If you need to access this collection from a derived type, you should + /// synchronize access to it using the protected field. + /// + protected readonly ISet m_staleFiles = new HashSet(); // Files written, but not yet sync'ed + + /// + /// A object to synchronize access to the collection. + /// You should synchronize access to using this object from derived types. + /// + protected readonly object syncLock = new object(); #pragma warning disable 612, 618 private int chunkSize = DEFAULT_READ_CHUNK_SIZE; @@ -299,28 +315,39 @@ public override long FileLength(string name) public override void DeleteFile(string name) { EnsureOpen(); - FileInfo file = new FileInfo(Path.Combine(m_directory.FullName, name)); - // LUCENENET specific: We need to explicitly throw when the file has already been deleted, - // since FileInfo doesn't do that for us. - // (An enhancement carried over from Lucene 8.2.0) - if (!File.Exists(file.FullName)) - { - throw new FileNotFoundException("Cannot delete " + file + " because it doesn't exist."); - } + string file = Path.Combine(m_directory.FullName, name); + + // LUCENENET Specific: See remarks for m_staleFiles field. + UninterruptableMonitor.Enter(syncLock); try { - file.Delete(); - if (File.Exists(file.FullName)) + // LUCENENET specific: We need to explicitly throw when the file has already been deleted, + // since FileInfo doesn't do that for us. + // (An enhancement carried over from Lucene 8.2.0) + if (!File.Exists(file)) + { + throw new FileNotFoundException("Cannot delete " + file + " because it doesn't exist."); + } + + try + { + File.Delete(file); + if (File.Exists(file)) + { + throw new IOException("Cannot delete " + file); + } + } + catch (Exception e) { - throw new IOException("Cannot delete " + file); + throw new IOException("Cannot delete " + file, e); } + + m_staleFiles.Remove(name); } - catch (Exception e) + finally { - throw new IOException("Cannot delete " + file, e); + UninterruptableMonitor.Exit(syncLock); } - - m_staleFiles.Remove(name); } /// @@ -363,7 +390,16 @@ protected virtual void EnsureCanWrite(string name) protected virtual void OnIndexOutputClosed(FSIndexOutput io) { - m_staleFiles.Add(io.name); + // LUCENENET Specific: See remarks for m_staleFiles field. + UninterruptableMonitor.Enter(syncLock); + try + { + m_staleFiles.Add(io.name); + } + finally + { + UninterruptableMonitor.Exit(syncLock); + } } public override void Sync(ICollection names) @@ -371,21 +407,31 @@ public override void Sync(ICollection names) EnsureOpen(); ISet toSync = new HashSet(names); - toSync.IntersectWith(m_staleFiles); - foreach (var name in toSync) + // LUCENENET Specific: See remarks for m_staleFiles field. + UninterruptableMonitor.Enter(syncLock); + try { - Fsync(name); - } + toSync.IntersectWith(m_staleFiles); - // fsync the directory itself, but only if there was any file fsynced before - // (otherwise it can happen that the directory does not yet exist)! - if (toSync.Count > 0) + foreach (var name in toSync) + { + Fsync(name); + } + + // fsync the directory itself, but only if there was any file fsynced before + // (otherwise it can happen that the directory does not yet exist)! + if (toSync.Count > 0) + { + IOUtils.Fsync(m_directory.FullName, true); + } + + m_staleFiles.ExceptWith(toSync); + } + finally { - IOUtils.Fsync(m_directory.FullName, true); + UninterruptableMonitor.Exit(syncLock); } - - m_staleFiles.ExceptWith(toSync); } public override string GetLockID() From 2af651bfba0b31e58df065a7ca050528d536b5f3 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Wed, 15 May 2024 20:28:24 -0600 Subject: [PATCH 14/17] Rename syncLock to m_syncLock --- src/Lucene.Net/Store/FSDirectory.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Lucene.Net/Store/FSDirectory.cs b/src/Lucene.Net/Store/FSDirectory.cs index 63d92f9cca..66950592c6 100644 --- a/src/Lucene.Net/Store/FSDirectory.cs +++ b/src/Lucene.Net/Store/FSDirectory.cs @@ -99,10 +99,10 @@ public abstract class FSDirectory : BaseDirectory /// /// /// LUCENENET NOTE: This is a non-thread-safe collection so that we can synchronize access to it - /// using the field. This is to prevent race conditions, i.e. one thread + /// using the field. This is to prevent race conditions, i.e. one thread /// adding a file to the collection while another thread is trying to sync the files, which could /// cause a missed sync. If you need to access this collection from a derived type, you should - /// synchronize access to it using the protected field. + /// synchronize access to it using the protected field. /// protected readonly ISet m_staleFiles = new HashSet(); // Files written, but not yet sync'ed @@ -110,7 +110,7 @@ public abstract class FSDirectory : BaseDirectory /// A object to synchronize access to the collection. /// You should synchronize access to using this object from derived types. /// - protected readonly object syncLock = new object(); + protected readonly object m_syncLock = new object(); #pragma warning disable 612, 618 private int chunkSize = DEFAULT_READ_CHUNK_SIZE; @@ -318,7 +318,7 @@ public override void DeleteFile(string name) string file = Path.Combine(m_directory.FullName, name); // LUCENENET Specific: See remarks for m_staleFiles field. - UninterruptableMonitor.Enter(syncLock); + UninterruptableMonitor.Enter(m_syncLock); try { // LUCENENET specific: We need to explicitly throw when the file has already been deleted, @@ -346,7 +346,7 @@ public override void DeleteFile(string name) } finally { - UninterruptableMonitor.Exit(syncLock); + UninterruptableMonitor.Exit(m_syncLock); } } @@ -391,14 +391,14 @@ protected virtual void EnsureCanWrite(string name) protected virtual void OnIndexOutputClosed(FSIndexOutput io) { // LUCENENET Specific: See remarks for m_staleFiles field. - UninterruptableMonitor.Enter(syncLock); + UninterruptableMonitor.Enter(m_syncLock); try { m_staleFiles.Add(io.name); } finally { - UninterruptableMonitor.Exit(syncLock); + UninterruptableMonitor.Exit(m_syncLock); } } @@ -409,7 +409,7 @@ public override void Sync(ICollection names) ISet toSync = new HashSet(names); // LUCENENET Specific: See remarks for m_staleFiles field. - UninterruptableMonitor.Enter(syncLock); + UninterruptableMonitor.Enter(m_syncLock); try { toSync.IntersectWith(m_staleFiles); @@ -430,7 +430,7 @@ public override void Sync(ICollection names) } finally { - UninterruptableMonitor.Exit(syncLock); + UninterruptableMonitor.Exit(m_syncLock); } } From b3a425f33ccef4656b08152fa532a0776d706671 Mon Sep 17 00:00:00 2001 From: Shad Storhaug Date: Sat, 25 May 2024 00:40:14 +0700 Subject: [PATCH 15/17] Lucene.Net.Index.TestIndexWriterOnJRECrash: Added try/finally block and refactored to ensure the TcpListener and Process are cleaned up at the end of each test iteration. This makes it run ~20% faster. --- .../Index/TestIndexWriterOnJRECrash.cs | 77 ++++++++++--------- 1 file changed, 41 insertions(+), 36 deletions(-) diff --git a/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs b/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs index 79b6ca15e0..62094329f3 100644 --- a/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs +++ b/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs @@ -13,6 +13,7 @@ using System.Reflection; using System.Threading; using BaseDirectoryWrapper = Lucene.Net.Store.BaseDirectoryWrapper; +using Assert = Lucene.Net.TestFramework.Assert; using Console = Lucene.Net.Util.SystemConsole; namespace Lucene.Net.Index @@ -42,7 +43,7 @@ namespace Lucene.Net.Index [TestFixture] public class TestIndexWriterOnJRECrash : TestNRTThreads { - // LUCENENET: Setup unnecessary because we create a new temp directory + // LUCENENET: Setup of directory unnecessary because we create a new temp directory // in each iteration of the test. [Test] @@ -64,33 +65,44 @@ public override void TestNRTThreads_Mem() DirectoryInfo tempDir = CreateTempDir("netcrash"); // Set up a TCP listener to receive the process ID - int port = SetupSocketListener(out TcpListener listener); + TcpListener listener = SetupSocketListener(); + Process p = null; + try + { + // Get the port that we picked at random. + int port = ((IPEndPoint)listener.LocalEndpoint).Port; - // Note this is the vstest.console process we are tracking here. - Process p = ForkTest(tempDir.FullName, port); + // Note this is the vstest.console process we are tracking here. + p = ForkTest(tempDir.FullName, port); - TextWriter childOut = BeginOutput(p, out ThreadJob stdOutPumper, out ThreadJob stdErrPumper); + TextWriter childOut = BeginOutput(p, out ThreadJob stdOutPumper, out ThreadJob stdErrPumper); - // LUCENENET: Note that ForkTest() creates the vstest.console.exe process. - // This spawns testhost.exe, which runs our test. We wait until - // the process starts and transmits its own Id so we know who to kill later. - int processIdToKill = WaitForProcessId(listener); + // LUCENENET: Note that ForkTest() creates the vstest.console.exe process. + // This spawns testhost.exe, which runs our test. We wait until + // the process starts and transmits its own PID so we know who to kill later. + int processIdToKill = WaitForProcessId(listener); - // Setup a time to crash the forked thread - int crashTime = TestUtil.NextInt32(Random, 4000, 5000); // LUCENENET: Adjusted these up by 1 second to give our tests some more time to spin up - ThreadJob t = new ThreadAnonymousClass(this, crashTime, processIdToKill); + // Setup a time to crash the forked thread + int crashTime = TestUtil.NextInt32(Random, 4000, 5000); // LUCENENET: Adjusted these up by 1 second to give our tests some more time to spin up + ThreadJob t = new ThreadAnonymousClass(this, crashTime, processIdToKill); - t.Priority = ThreadPriority.Highest; - t.Start(); - t.Join(); // Wait for our thread to kill the other process + t.Priority = ThreadPriority.Highest; + t.Start(); + t.Join(); // Wait for our thread to kill the other process - // if we succeeded in finding an index, we are done. - if (CheckIndexes(tempDir)) - { + // if we succeeded in finding an index, we are done. + if (CheckIndexes(tempDir)) + { + EndOutput(p, childOut, stdOutPumper, stdErrPumper); + return; + } EndOutput(p, childOut, stdOutPumper, stdErrPumper); - return; } - EndOutput(p, childOut, stdOutPumper, stdErrPumper); + finally + { + listener.Stop(); + p?.Dispose(); + } } } else @@ -100,7 +112,7 @@ public override void TestNRTThreads_Mem() // we are the fork, log our processId so the original test can kill us. int processIdToKill = Process.GetCurrentProcess().Id; - int port = SystemProperties.GetPropertyAsInt32("tests:crashtestport", -1); + int port = SystemProperties.GetPropertyAsInt32("tests:crashtestport"); assertTrue("No tests:crashtestport value was passed to the fork. This is a required system property.", port > 0); @@ -334,29 +346,22 @@ public virtual bool CheckIndexes(FileSystemInfo file) return false; } - private int SetupSocketListener(out TcpListener listener) + private TcpListener SetupSocketListener() { // Pick a random port that is available on the local machine. - listener = new TcpListener(IPAddress.Loopback, 0); + TcpListener listener = new TcpListener(IPAddress.Loopback, 0); listener.Start(); - return ((IPEndPoint)listener.LocalEndpoint).Port; + return listener; } // LUCENENET: Wait for our test to spin up and send its process ID so we can kill it. private int WaitForProcessId(TcpListener listener) { - try - { - using var client = listener.AcceptTcpClient(); - using var stream = client.GetStream(); - // Directly read the process ID as a 32-bit integer - using var reader = new BinaryReader(stream); - return reader.ReadInt32(); - } - finally - { - listener.Stop(); - } + using var client = listener.AcceptTcpClient(); + using var stream = client.GetStream(); + // Directly read the process ID as a 32-bit integer + using var reader = new BinaryReader(stream); + return reader.ReadInt32(); } private void SendProcessId(int processId, int port) From a79425b894fbb489d5e4b96ca5138015d663bd61 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Mon, 12 Aug 2024 09:06:58 -0600 Subject: [PATCH 16/17] Refactor rename namespace to Lucene.Net.Native --- src/Lucene.Net/Support/IO/PosixFsyncSupport.cs | 4 ++-- src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs | 2 +- src/Lucene.Net/Support/Native/Interop.MacOS.Constants.cs | 2 +- src/Lucene.Net/Support/Native/Interop.Posix.Close.cs | 2 +- src/Lucene.Net/Support/Native/Interop.Posix.Constants.cs | 2 +- src/Lucene.Net/Support/Native/Interop.Posix.Fcntl.cs | 2 +- src/Lucene.Net/Support/Native/Interop.Posix.Fsync.cs | 2 +- src/Lucene.Net/Support/Native/Interop.Posix.Open.cs | 2 +- src/Lucene.Net/Support/Native/Interop.Win32.CloseHandle.cs | 2 +- src/Lucene.Net/Support/Native/Interop.Win32.Constants.cs | 2 +- src/Lucene.Net/Support/Native/Interop.Win32.CreateFileW.cs | 2 +- .../Support/Native/Interop.Win32.FlushFileBuffers.cs | 2 +- 12 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs b/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs index 352739ff49..d5f4098404 100644 --- a/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs +++ b/src/Lucene.Net/Support/IO/PosixFsyncSupport.cs @@ -2,8 +2,8 @@ using System; using System.IO; using System.Runtime.InteropServices; -using static Lucene.Net.Support.Native.Interop.Posix; -using static Lucene.Net.Support.Native.Interop.MacOS; +using static Lucene.Net.Native.Interop.Posix; +using static Lucene.Net.Native.Interop.MacOS; namespace Lucene.Net.Support.IO { diff --git a/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs b/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs index 78017a20fa..ace2ab76af 100644 --- a/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs +++ b/src/Lucene.Net/Support/IO/WindowsFsyncSupport.cs @@ -1,7 +1,7 @@ using System; using System.IO; using System.Runtime.InteropServices; -using static Lucene.Net.Support.Native.Interop.Win32; +using static Lucene.Net.Native.Interop.Win32; namespace Lucene.Net.Support.IO { diff --git a/src/Lucene.Net/Support/Native/Interop.MacOS.Constants.cs b/src/Lucene.Net/Support/Native/Interop.MacOS.Constants.cs index b0ba4613d8..d40b745535 100644 --- a/src/Lucene.Net/Support/Native/Interop.MacOS.Constants.cs +++ b/src/Lucene.Net/Support/Native/Interop.MacOS.Constants.cs @@ -1,4 +1,4 @@ -namespace Lucene.Net.Support.Native +namespace Lucene.Net.Native { /* * Licensed to the Apache Software Foundation (ASF) under one or more diff --git a/src/Lucene.Net/Support/Native/Interop.Posix.Close.cs b/src/Lucene.Net/Support/Native/Interop.Posix.Close.cs index 009b0daf45..d4fb9a7b99 100644 --- a/src/Lucene.Net/Support/Native/Interop.Posix.Close.cs +++ b/src/Lucene.Net/Support/Native/Interop.Posix.Close.cs @@ -1,6 +1,6 @@ using System.Runtime.InteropServices; -namespace Lucene.Net.Support.Native +namespace Lucene.Net.Native { /* * Licensed to the Apache Software Foundation (ASF) under one or more diff --git a/src/Lucene.Net/Support/Native/Interop.Posix.Constants.cs b/src/Lucene.Net/Support/Native/Interop.Posix.Constants.cs index d483a7d9ff..ef8c02ac36 100644 --- a/src/Lucene.Net/Support/Native/Interop.Posix.Constants.cs +++ b/src/Lucene.Net/Support/Native/Interop.Posix.Constants.cs @@ -1,4 +1,4 @@ -namespace Lucene.Net.Support.Native +namespace Lucene.Net.Native { /* * Licensed to the Apache Software Foundation (ASF) under one or more diff --git a/src/Lucene.Net/Support/Native/Interop.Posix.Fcntl.cs b/src/Lucene.Net/Support/Native/Interop.Posix.Fcntl.cs index 42a6eb9135..a9e20d5c10 100644 --- a/src/Lucene.Net/Support/Native/Interop.Posix.Fcntl.cs +++ b/src/Lucene.Net/Support/Native/Interop.Posix.Fcntl.cs @@ -1,6 +1,6 @@ using System.Runtime.InteropServices; -namespace Lucene.Net.Support.Native +namespace Lucene.Net.Native { /* * Licensed to the Apache Software Foundation (ASF) under one or more diff --git a/src/Lucene.Net/Support/Native/Interop.Posix.Fsync.cs b/src/Lucene.Net/Support/Native/Interop.Posix.Fsync.cs index f63116596d..eafb1715f0 100644 --- a/src/Lucene.Net/Support/Native/Interop.Posix.Fsync.cs +++ b/src/Lucene.Net/Support/Native/Interop.Posix.Fsync.cs @@ -1,6 +1,6 @@ using System.Runtime.InteropServices; -namespace Lucene.Net.Support.Native +namespace Lucene.Net.Native { /* * Licensed to the Apache Software Foundation (ASF) under one or more diff --git a/src/Lucene.Net/Support/Native/Interop.Posix.Open.cs b/src/Lucene.Net/Support/Native/Interop.Posix.Open.cs index 0ef5b9c49e..b66409ee7c 100644 --- a/src/Lucene.Net/Support/Native/Interop.Posix.Open.cs +++ b/src/Lucene.Net/Support/Native/Interop.Posix.Open.cs @@ -1,6 +1,6 @@ using System.Runtime.InteropServices; -namespace Lucene.Net.Support.Native +namespace Lucene.Net.Native { /* * Licensed to the Apache Software Foundation (ASF) under one or more diff --git a/src/Lucene.Net/Support/Native/Interop.Win32.CloseHandle.cs b/src/Lucene.Net/Support/Native/Interop.Win32.CloseHandle.cs index 9c8f56fda7..fa4dafb919 100644 --- a/src/Lucene.Net/Support/Native/Interop.Win32.CloseHandle.cs +++ b/src/Lucene.Net/Support/Native/Interop.Win32.CloseHandle.cs @@ -1,7 +1,7 @@ using System; using System.Runtime.InteropServices; -namespace Lucene.Net.Support.Native +namespace Lucene.Net.Native { /* * Licensed to the Apache Software Foundation (ASF) under one or more diff --git a/src/Lucene.Net/Support/Native/Interop.Win32.Constants.cs b/src/Lucene.Net/Support/Native/Interop.Win32.Constants.cs index 313dec7e19..2279c02c9a 100644 --- a/src/Lucene.Net/Support/Native/Interop.Win32.Constants.cs +++ b/src/Lucene.Net/Support/Native/Interop.Win32.Constants.cs @@ -1,6 +1,6 @@ using System; -namespace Lucene.Net.Support.Native +namespace Lucene.Net.Native { /* * Licensed to the Apache Software Foundation (ASF) under one or more diff --git a/src/Lucene.Net/Support/Native/Interop.Win32.CreateFileW.cs b/src/Lucene.Net/Support/Native/Interop.Win32.CreateFileW.cs index 8c52113dc9..e6732c0b2c 100644 --- a/src/Lucene.Net/Support/Native/Interop.Win32.CreateFileW.cs +++ b/src/Lucene.Net/Support/Native/Interop.Win32.CreateFileW.cs @@ -1,7 +1,7 @@ using System; using System.Runtime.InteropServices; -namespace Lucene.Net.Support.Native +namespace Lucene.Net.Native { /* * Licensed to the Apache Software Foundation (ASF) under one or more diff --git a/src/Lucene.Net/Support/Native/Interop.Win32.FlushFileBuffers.cs b/src/Lucene.Net/Support/Native/Interop.Win32.FlushFileBuffers.cs index ad448a9795..c47ffa95d3 100644 --- a/src/Lucene.Net/Support/Native/Interop.Win32.FlushFileBuffers.cs +++ b/src/Lucene.Net/Support/Native/Interop.Win32.FlushFileBuffers.cs @@ -1,7 +1,7 @@ using System; using System.Runtime.InteropServices; -namespace Lucene.Net.Support.Native +namespace Lucene.Net.Native { /* * Licensed to the Apache Software Foundation (ASF) under one or more From 4118e2f85e50c2e467a02482ed74b8aac444628a Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Mon, 12 Aug 2024 09:09:51 -0600 Subject: [PATCH 17/17] Mark JRE crash test as [AwaitsFix] --- src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs b/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs index 62094329f3..9056852124 100644 --- a/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs +++ b/src/Lucene.Net.Tests/Index/TestIndexWriterOnJRECrash.cs @@ -48,7 +48,7 @@ public class TestIndexWriterOnJRECrash : TestNRTThreads [Test] [Slow] - [Repeat(25)] + [AwaitsFix] public override void TestNRTThreads_Mem() { //if we are not the fork