From 383fc2d635875472cd37ee6d2162f4dce3568792 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Wed, 16 Dec 2009 12:01:45 +0000 Subject: [PATCH] LUCENE-2104: Fix NativeFSLock.release() to throw exception if lock is held by another thread/process. git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@891205 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 3 +++ .../lucene/store/NativeFSLockFactory.java | 20 ++++++++++++++++++- .../apache/lucene/store/TestLockFactory.java | 20 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 0b3ed5dee42..31594f0971c 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -56,6 +56,9 @@ Bug fixes * LUCENE-2166: Don't incorrectly keep warning about the same immense term, when IndexWriter.infoStream is on. (Mike McCandless) + +* LUCENE-2104: NativeFSLock.release() would silently fail if the lock is held by + another thread/process. (Shai Erera via Uwe Schindler) New features diff --git a/src/java/org/apache/lucene/store/NativeFSLockFactory.java b/src/java/org/apache/lucene/store/NativeFSLockFactory.java index fa813d46900..bbab8d1b2cd 100755 --- a/src/java/org/apache/lucene/store/NativeFSLockFactory.java +++ b/src/java/org/apache/lucene/store/NativeFSLockFactory.java @@ -146,7 +146,7 @@ public class NativeFSLockFactory extends FSLockFactory { } } } -}; +} class NativeFSLock extends Lock { @@ -300,6 +300,24 @@ class NativeFSLock extends Lock { } if (!path.delete()) throw new LockReleaseFailedException("failed to delete " + path); + } else { + // if we don't hold the lock, and somebody still called release(), for + // example as a result of calling IndexWriter.unlock(), we should attempt + // to obtain the lock and release it. If the obtain fails, it means the + // lock cannot be released, and we should throw a proper exception rather + // than silently failing/not doing anything. + boolean obtained = false; + try { + if (!(obtained = obtain())) { + throw new LockReleaseFailedException( + "Cannot forcefully unlock a NativeFSLock which is held by another indexer component: " + + path); + } + } finally { + if (obtained) { + release(); + } + } } } diff --git a/src/test/org/apache/lucene/store/TestLockFactory.java b/src/test/org/apache/lucene/store/TestLockFactory.java index 535e5c142b8..6f649a827ce 100755 --- a/src/test/org/apache/lucene/store/TestLockFactory.java +++ b/src/test/org/apache/lucene/store/TestLockFactory.java @@ -198,6 +198,26 @@ public class TestLockFactory extends LuceneTestCase { assertFalse(l2.isLocked()); } + public void testNativeFSLockReleaseByOtherLock() throws IOException { + + NativeFSLockFactory f = new NativeFSLockFactory(System.getProperty("tempDir")); + + f.setLockPrefix("test"); + Lock l = f.makeLock("commit"); + Lock l2 = f.makeLock("commit"); + + assertTrue("failed to obtain lock", l.obtain()); + try { + assertTrue(l2.isLocked()); + l2.release(); + fail("should not have reached here. LockReleaseFailedException should have been thrown"); + } catch (IOException e) { + assertTrue("Unexpected exception", e instanceof LockReleaseFailedException); + } finally { + l.release(); + } + } + // Verify: NativeFSLockFactory assigns null as lockPrefix if the lockDir is inside directory public void testNativeFSLockFactoryPrefix() throws IOException {