diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 79b27e9d4051..5211861a7034 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -200,6 +200,8 @@ Bug Fixes * GITHUB#16278: Refactor NativeFSLockFactory lock acquisition to cleanly separate locking and file system logic. (Bharathi-Kanna) +* GITHUB#16303: Add suppressed exception details when lock acquisition fails in NativeFSLockFactory. (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 056bd3cf01a0..15ceed4c3e03 100644 --- a/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java +++ b/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java @@ -128,19 +128,17 @@ protected Lock obtainFSLock(FSDirectory dir, String lockName) throws IOException */ 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(); + FileLock 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 - } + } catch (Throwable t) { + IOUtils.closeWhileSuppressingExceptions(t, channel); + throw t; } } 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(); + } }