From 6e1951e14f78b9139ad629e90a4fe82d83c818d7 Mon Sep 17 00:00:00 2001 From: Bharathi-Kanna <99189546+Bharathi-Kanna@users.noreply.github.com> Date: Sat, 20 Jun 2026 18:37:13 +0530 Subject: [PATCH 1/3] Add suppressed exception details in NativeFSLockFactory lock acquisition failure When lock acquisition fails in NativeFSLockFactory.obtainFSLock, exceptions thrown while closing the FileChannel are now attached as suppressed exceptions to the original failure instead of being silently swallowed via IOUtils.closeWhileHandlingException. Changes: - Replace finally block with catch(Throwable) to properly propagate suppressed exceptions using IOUtils.closeWhileSuppressingExceptions and addSuppressed - Add test testSuppressedExceptionOnLockFailure verifying close failures appear as suppressed exceptions on the original lock failure - Add CHANGES.txt entry under Lucene 11.0.0 Bug Fixes Resolves the TODO comment at NativeFSLockFactory.java line 121. --- lucene/CHANGES.txt | 5 ++ .../lucene/store/NativeFSLockFactory.java | 13 +++-- .../lucene/store/TestNativeFSLockFactory.java | 53 +++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 42a6e8073576..d39953072886 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -195,6 +195,11 @@ Bug Fixes * GITHUB#15939: Fix thread-safety issues with NFARunAutomaton. (Dimitris Rempapis) +* GITHUB#: Add suppressed exception details when lock acquisition fails in + NativeFSLockFactory. Exceptions thrown while closing the FileChannel on lock + failure are now attached as suppressed exceptions instead of being silently + swallowed, resolving a long-standing TODO. (Bharathi-Kanna) + Changes in Runtime Behavior --------------------- * GITHUB#14187: The query cache is now disabled by default. (Adrien Grand) diff --git a/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java b/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java index 909b98e135dc..b0a0916a0c77 100644 --- a/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java +++ b/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java @@ -116,11 +116,16 @@ protected Lock obtainFSLock(FSDirectory dir, String lockName) throws IOException } else { throw new LockObtainFailedException("Lock held by another program: " + realPath); } - } finally { - if (lock == null) { // not successful - clear up and move out - IOUtils.closeWhileHandlingException(channel); // TODO: addSuppressed - clearLockHeld(realPath); // clear LOCK_HELD last + } catch (Throwable t) { + if (channel != null) { + IOUtils.closeWhileSuppressingExceptions(t, channel); + } + try { + clearLockHeld(realPath); + } catch (Throwable t2) { + t.addSuppressed(t2); } + throw t; } } else { throw new LockObtainFailedException("Lock held by this virtual machine: " + realPath); diff --git a/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java b/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java index 0c87b5a0a0d3..88c3aa0c522a 100644 --- a/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java +++ b/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java @@ -17,6 +17,8 @@ package org.apache.lucene.store; import java.io.IOException; +import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; import java.nio.channels.SeekableByteChannel; import java.nio.file.AccessDeniedException; import java.nio.file.FileSystem; @@ -25,6 +27,7 @@ import java.nio.file.Path; import java.nio.file.attribute.FileAttribute; import java.util.Set; +import org.apache.lucene.tests.mockfile.FilterFileChannel; import org.apache.lucene.tests.mockfile.FilterFileSystemProvider; import org.apache.lucene.tests.mockfile.FilterPath; import org.apache.lucene.tests.store.BaseLockFactoryTestCase; @@ -143,4 +146,54 @@ public void testBadPermissions() throws IOException { dir.close(); } + + /** MockFileSystem that throws exception on lock and close */ + static class MockFailingCloseFileSystem extends FilterFileSystemProvider { + public MockFailingCloseFileSystem(FileSystem delegateInstance) { + super("mockfailingclose://", delegateInstance); + } + + @Override + public FileChannel newFileChannel( + Path path, Set options, FileAttribute... attrs) + throws IOException { + if (path.getFileName().toString().equals("test.lock")) { + return new FilterFileChannel(super.newFileChannel(path, options, attrs)) { + @Override + public FileLock tryLock(long position, long size, boolean shared) throws IOException { + throw new IOException("fake lock failure"); + } + + @Override + protected void implCloseChannel() throws IOException { + super.implCloseChannel(); + throw new IOException("fake close failure"); + } + }; + } + return super.newFileChannel(path, options, attrs); + } + } + + public void testSuppressedExceptionOnLockFailure() throws IOException { + Path tmpDir = createTempDir(); + tmpDir = FilterPath.unwrap(tmpDir).toRealPath(); + FileSystem mock = new MockFailingCloseFileSystem(tmpDir.getFileSystem()).getFileSystem(null); + Path mockPath = mock.getPath(tmpDir.toString()); + + Directory dir = getDirectory(mockPath.resolve("indexDir")); + IOException expected = + expectThrows( + IOException.class, + () -> { + dir.obtainLock("test.lock"); + }); + + assertEquals("fake lock failure", expected.getMessage()); + Throwable[] suppressed = expected.getSuppressed(); + assertEquals(1, suppressed.length); + assertEquals("fake close failure", suppressed[0].getMessage()); + + dir.close(); + } } From 8c6950e61bbaa84fbf9a66d0957bfc63e8d01de0 Mon Sep 17 00:00:00 2001 From: Bharathi-Kanna <99189546+Bharathi-Kanna@users.noreply.github.com> Date: Sat, 20 Jun 2026 18:41:04 +0530 Subject: [PATCH 2/3] Update CHANGES.txt with PR number #16278 --- lucene/CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index d39953072886..0e51c4bead20 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -195,7 +195,7 @@ Bug Fixes * GITHUB#15939: Fix thread-safety issues with NFARunAutomaton. (Dimitris Rempapis) -* GITHUB#: Add suppressed exception details when lock acquisition fails in +* GITHUB#16278: Add suppressed exception details when lock acquisition fails in NativeFSLockFactory. Exceptions thrown while closing the FileChannel on lock failure are now attached as suppressed exceptions instead of being silently swallowed, resolving a long-standing TODO. (Bharathi-Kanna) From d0380f7a38b3fc53aae5a706232b1c9027ac779e Mon Sep 17 00:00:00 2001 From: Bharathi-Kanna <99189546+Bharathi-Kanna@users.noreply.github.com> Date: Sun, 21 Jun 2026 10:35:15 +0530 Subject: [PATCH 3/3] Refactor NativeFSLockFactory lock acquisition to separate concerns --- lucene/CHANGES.txt | 5 +- .../lucene/store/NativeFSLockFactory.java | 46 ++++++++++------ .../lucene/store/TestNativeFSLockFactory.java | 53 ------------------- 3 files changed, 30 insertions(+), 74 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 0e51c4bead20..b244b83e9bf6 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -195,10 +195,7 @@ Bug Fixes * GITHUB#15939: Fix thread-safety issues with NFARunAutomaton. (Dimitris Rempapis) -* GITHUB#16278: Add suppressed exception details when lock acquisition fails in - NativeFSLockFactory. Exceptions thrown while closing the FileChannel on lock - failure are now attached as suppressed exceptions instead of being silently - swallowed, resolving a long-standing TODO. (Bharathi-Kanna) +* GITHUB#16278: Refactor NativeFSLockFactory lock acquisition to cleanly separate locking and file system logic. (Bharathi-Kanna) Changes in Runtime Behavior --------------------- diff --git a/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java b/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java index b0a0916a0c77..056bd3cf01a0 100644 --- a/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java +++ b/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java @@ -106,32 +106,44 @@ protected Lock obtainFSLock(FSDirectory dir, String lockName) throws IOException Files.readAttributes(realPath, BasicFileAttributes.class).creationTime(); if (LOCK_HELD.add(realPath.toString())) { - FileChannel channel = null; - FileLock lock = null; + NativeFSLock lock = null; try { - channel = FileChannel.open(realPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE); - lock = channel.tryLock(); - if (lock != null) { - return new NativeFSLock(lock, channel, realPath, creationTime); - } else { - throw new LockObtainFailedException("Lock held by another program: " + realPath); - } - } catch (Throwable t) { - if (channel != null) { - IOUtils.closeWhileSuppressingExceptions(t, channel); - } - try { + lock = tryLock(realPath, creationTime); + return lock; + } finally { + if (lock == null) { clearLockHeld(realPath); - } catch (Throwable t2) { - t.addSuppressed(t2); } - throw t; } } else { throw new LockObtainFailedException("Lock held by this virtual machine: " + realPath); } } + /** + * Attempts to obtain the lock without blocking. + * + * @return a NativeFSLock if successful + * @throws IOException if not successful + */ + private static NativeFSLock tryLock(Path path, FileTime creationTime) throws IOException { + FileChannel channel = null; + FileLock lock = null; + try { + channel = FileChannel.open(path, StandardOpenOption.CREATE, StandardOpenOption.WRITE); + lock = channel.tryLock(); + if (lock != null) { + return new NativeFSLock(lock, channel, path, creationTime); + } else { + throw new LockObtainFailedException("Lock held by another program: " + path); + } + } finally { + if (lock == null) { // not successful - clear up and move out + IOUtils.closeWhileHandlingException(channel); // TODO: addSuppressed + } + } + } + private static void clearLockHeld(Path path) throws IOException { boolean remove = LOCK_HELD.remove(path.toString()); if (remove == false) { diff --git a/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java b/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java index 88c3aa0c522a..0c87b5a0a0d3 100644 --- a/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java +++ b/lucene/core/src/test/org/apache/lucene/store/TestNativeFSLockFactory.java @@ -17,8 +17,6 @@ package org.apache.lucene.store; import java.io.IOException; -import java.nio.channels.FileChannel; -import java.nio.channels.FileLock; import java.nio.channels.SeekableByteChannel; import java.nio.file.AccessDeniedException; import java.nio.file.FileSystem; @@ -27,7 +25,6 @@ import java.nio.file.Path; import java.nio.file.attribute.FileAttribute; import java.util.Set; -import org.apache.lucene.tests.mockfile.FilterFileChannel; import org.apache.lucene.tests.mockfile.FilterFileSystemProvider; import org.apache.lucene.tests.mockfile.FilterPath; import org.apache.lucene.tests.store.BaseLockFactoryTestCase; @@ -146,54 +143,4 @@ public void testBadPermissions() throws IOException { dir.close(); } - - /** MockFileSystem that throws exception on lock and close */ - static class MockFailingCloseFileSystem extends FilterFileSystemProvider { - public MockFailingCloseFileSystem(FileSystem delegateInstance) { - super("mockfailingclose://", delegateInstance); - } - - @Override - public FileChannel newFileChannel( - Path path, Set options, FileAttribute... attrs) - throws IOException { - if (path.getFileName().toString().equals("test.lock")) { - return new FilterFileChannel(super.newFileChannel(path, options, attrs)) { - @Override - public FileLock tryLock(long position, long size, boolean shared) throws IOException { - throw new IOException("fake lock failure"); - } - - @Override - protected void implCloseChannel() throws IOException { - super.implCloseChannel(); - throw new IOException("fake close failure"); - } - }; - } - return super.newFileChannel(path, options, attrs); - } - } - - public void testSuppressedExceptionOnLockFailure() throws IOException { - Path tmpDir = createTempDir(); - tmpDir = FilterPath.unwrap(tmpDir).toRealPath(); - FileSystem mock = new MockFailingCloseFileSystem(tmpDir.getFileSystem()).getFileSystem(null); - Path mockPath = mock.getPath(tmpDir.toString()); - - Directory dir = getDirectory(mockPath.resolve("indexDir")); - IOException expected = - expectThrows( - IOException.class, - () -> { - dir.obtainLock("test.lock"); - }); - - assertEquals("fake lock failure", expected.getMessage()); - Throwable[] suppressed = expected.getSuppressed(); - assertEquals(1, suppressed.length); - assertEquals("fake close failure", suppressed[0].getMessage()); - - dir.close(); - } }