LUCENE-2421: Hardening of NativeFSLock

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@940730 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -141,6 +141,10 @@ Changes in runtime behavior
 * LUCENE-2179: CharArraySet.clear() is now functional.
   (Robert Muir, Uwe Schindler)
 
+* LUCENE-2421: NativeFSLockFactory does not throw LockReleaseFailedException if 
+  it cannot delete the lock file, since obtaining the lock does not fail if the 
+  file is there. (Shai Erera)
+
 API Changes
 
 * LUCENE-2076: Rename FSDirectory.getFile -> getDirectory.  (George

lucene/src/java/org/apache/lucene/store/NativeFSLockFactory.java

@@ -17,6 +17,7 @@ package org.apache.lucene.store;
  * limitations under the License.
  */
 
+import java.lang.management.ManagementFactory;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
 import java.io.File;
@@ -78,12 +79,26 @@ public class NativeFSLockFactory extends FSLockFactory {
                             lockDir.getAbsolutePath());
     }
 
-    String randomLockName = "lucene-" + Long.toString(new Random().nextInt(), Character.MAX_RADIX) + "-test.lock";
+    // add the RuntimeMXBean's name to the lock file, to reduce the chance for
+    // name collisions when this code is invoked by multiple JVMs (such as in
+    // our tests). On most systems, the name includes the process Id.
+    // Also, remove any non-alphanumeric characters, so that the lock file will
+    // be created for sure on all systems.
+    String randomLockName = "lucene-"
+        + ManagementFactory.getRuntimeMXBean().getName().replaceAll("[^a..zA..Z0..9]+","") + "-"
+        + Long.toString(new Random().nextInt(), Character.MAX_RADIX)
+        + "-test.lock";
     
     Lock l = makeLock(randomLockName);
     try {
       l.obtain();
       l.release();
+      // If the test lock failed to delete after all the attempts, attempt a
+      // delete when the JVM exits.
+      File lockFile = new File(lockDir, randomLockName);
+      if (lockFile.exists()) {
+        lockFile.deleteOnExit();
+      }
     } catch (IOException e) {
       RuntimeException e2 = new RuntimeException("Failed to acquire random test lock; please verify filesystem for lock directory '" + lockDir + "' supports locking");
       e2.initCause(e);
@@ -307,8 +322,10 @@ class NativeFSLock extends Lock {
           }
         }
       }
-      if (!path.delete())
-        throw new LockReleaseFailedException("failed to delete " + path);
+      // LUCENE-2421: we don't care anymore if the file cannot be deleted
+      // because it's held up by another process (e.g. AntiVirus). NativeFSLock
+      // does not depend on the existence/absence of the lock file
+      path.delete();
     } 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

lucene/src/test/org/apache/lucene/store/TestLockFactory.java

@@ -193,6 +193,22 @@ public class TestLockFactory extends LuceneTestCase {
       assertFalse(l2.isLocked());
     }
 
+    
+    // Verify: NativeFSLockFactory works correctly if the lock file exists
+    public void testNativeFSLockFactoryLockExists() throws IOException {
+      
+      File lockFile = new File(TEMP_DIR, "test.lock");
+      lockFile.createNewFile();
+      
+      Lock l = new NativeFSLockFactory(TEMP_DIR).makeLock("test.lock");
+      assertTrue("failed to obtain lock", l.obtain());
+      l.release();
+      assertFalse("failed to release lock", l.isLocked());
+      if (lockFile.exists()) {
+        lockFile.delete();
+      }
+    }
+
     public void testNativeFSLockReleaseByOtherLock() throws IOException {
 
       NativeFSLockFactory f = new NativeFSLockFactory(TEMP_DIR);
@@ -206,8 +222,8 @@ public class TestLockFactory extends LuceneTestCase {
         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);
+      } catch (LockReleaseFailedException e) {
+        // expected
       } finally {
         l.release();
       }