You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by rm...@apache.org on 2014/04/22 16:17:41 UTC
svn commit: r1589134 - in /lucene/dev/branches/branch_4x: ./ lucene/
lucene/core/ lucene/core/src/java/org/apache/lucene/store/
lucene/core/src/test/org/apache/lucene/store/
Author: rmuir
Date: Tue Apr 22 14:17:40 2014
New Revision: 1589134
URL: http://svn.apache.org/r1589134
Log:
LUCENE-5624: fix NativeFS file handle leak, improve lock testing
Modified:
lucene/dev/branches/branch_4x/ (props changed)
lucene/dev/branches/branch_4x/lucene/ (props changed)
lucene/dev/branches/branch_4x/lucene/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_4x/lucene/core/ (props changed)
lucene/dev/branches/branch_4x/lucene/core/build.xml
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java
lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/store/TestLockFactory.java
Modified: lucene/dev/branches/branch_4x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/CHANGES.txt?rev=1589134&r1=1589133&r2=1589134&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/lucene/CHANGES.txt Tue Apr 22 14:17:40 2014
@@ -232,6 +232,9 @@ Bug fixes
you may see write.lock hanging around from time to time: its harmless.
(Uwe Schindler, Mike McCandless, Robert Muir)
+* LUCENE-5624: Ensure NativeFSLockFactory does not leak file handles if it is unable
+ to obtain the lock. (Uwe Schindler, Robert Muir)
+
Test Framework
* LUCENE-5592: Incorrectly reported uncloseable files. (Dawid Weiss)
@@ -259,9 +262,7 @@ Build
by adding a workaround for the Ant bug. (Uwe Schindler)
* LUCENE-5612: Add a new Ant target in lucene/core to test LockFactory
- implementations: "ant test-lock-factory". By default it is ran
- after the core testsuite on NativeFSLockFactory. To test another one,
- pass -Dlock.factory.impl to Ant. (Uwe Schindler, Mike McCandless,
+ implementations: "ant test-lock-factory". (Uwe Schindler, Mike McCandless,
Robert Muir)
Documentation
Modified: lucene/dev/branches/branch_4x/lucene/core/build.xml
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/build.xml?rev=1589134&r1=1589133&r2=1589134&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/build.xml (original)
+++ lucene/dev/branches/branch_4x/lucene/core/build.xml Tue Apr 22 14:17:40 2014
@@ -148,6 +148,8 @@
<macrodef name="startLockStressTestClient">
<attribute name="clientId"/>
+ <attribute name="lockFactoryImpl"/>
+ <attribute name="lockFactoryDir"/>
<sequential>
<local name="lockverifyserver.port"/>
<groovy><![CDATA[
@@ -157,18 +159,42 @@
}
properties["lockverifyserver.port"] = port;
]]></groovy>
- <java taskname="LockStressTest@{clientId}" fork="true" classpathref="test-lock.classpath" classname="org.apache.lucene.store.LockStressTest" failOnError="true">
+ <java taskname="lockStressTest@{clientId}" fork="true" classpathref="test-lock.classpath" classname="org.apache.lucene.store.LockStressTest" failOnError="true">
<arg value="@{clientId}"/>
<arg value="${lockverifyserver.host}"/>
<arg value="${lockverifyserver.port}"/>
- <arg value="${lock.factory.impl}"/>
- <arg value="${lock.factory.dir}"/>
+ <arg value="@{lockFactoryImpl}"/>
+ <arg value="@{lockFactoryDir}"/>
<arg value="${lockverify.delay}"/>
<arg value="${lockverify.count}"/>
</java>
</sequential>
</macrodef>
+ <macrodef name="testLockFactory">
+ <attribute name="lockFactoryImpl"/>
+ <attribute name="lockFactoryDir"/>
+ <sequential>
+ <echo taskname="testLockFactory" message="Testing @{lockFactoryImpl}..."/>
+ <mkdir dir="@{lockFactoryDir}"/>
+ <parallel threadCount="3" failonany="false">
+ <sequential>
+ <!-- the server runs in-process, so we can wait for the sysproperty -->
+ <java taskname="lockVerifyServer" fork="false" classpathref="test-lock.classpath" classname="org.apache.lucene.store.LockVerifyServer" failOnError="true">
+ <arg value="${lockverifyserver.host}"/>
+ <arg value="2"/>
+ </java>
+ </sequential>
+ <sequential>
+ <startLockStressTestClient clientId="1" lockFactoryImpl="@{lockFactoryImpl}" lockFactoryDir="@{lockFactoryDir}" />
+ </sequential>
+ <sequential>
+ <startLockStressTestClient clientId="2" lockFactoryImpl="@{lockFactoryImpl}" lockFactoryDir="@{lockFactoryDir}" />
+ </sequential>
+ </parallel>
+ </sequential>
+ </macrodef>
+
<!-- we ignore our ant-based lock factory test, if user applies test filtering: -->
<condition property="-ignore-test-lock-factory">
<or>
@@ -180,21 +206,19 @@
<target name="test-lock-factory" depends="resolve-groovy,compile-core" unless="-ignore-test-lock-factory"
description="Run LockStressTest with multiple JVMs">
<property name="lockverifyserver.host" value="127.0.0.1"/>
- <property name="lock.factory.impl" value="org.apache.lucene.store.NativeFSLockFactory"/>
- <property name="lock.factory.dir" location="${build.dir}/lockfactorytest"/>
<property name="lockverify.delay" value="1"/>
- <groovy taskname="LockVerifySetup"><![CDATA[
+ <groovy taskname="lockVerifySetup"><![CDATA[
System.clearProperty("lockverifyserver.port"); // make sure it is undefined
if (!properties["lockverify.count"]) {
int count = Boolean.parseBoolean(properties["tests.nightly"]) ?
- 20000 : 2000;
+ 30000 : 2000;
count *= Integer.parseInt(properties["tests.multiplier"]);
properties["lockverify.count"] = count;
}
task.log("Configuration properties:");
- ["lock.factory.impl", "lockverify.delay", "lockverify.count"].each {
+ ["lockverify.delay", "lockverify.count"].each {
k -> task.log(" " + k + "=" + properties[k]);
}
]]></groovy>
@@ -202,22 +226,8 @@
<path refid="classpath"/>
<pathelement location="${build.dir}/classes/java"/>
</path>
- <mkdir dir="${lock.factory.dir}"/>
- <parallel threadCount="3" failonany="false">
- <sequential>
- <!-- the server runs in-process, so we can wait for the sysproperty -->
- <java taskname="LockVerifyServer" fork="false" classpathref="test-lock.classpath" classname="org.apache.lucene.store.LockVerifyServer" failOnError="true">
- <arg value="${lockverifyserver.host}"/>
- <arg value="2"/>
- </java>
- </sequential>
- <sequential>
- <startLockStressTestClient clientId="1"/>
- </sequential>
- <sequential>
- <startLockStressTestClient clientId="2"/>
- </sequential>
- </parallel>
+ <testLockFactory lockFactoryImpl="org.apache.lucene.store.NativeFSLockFactory" lockFactoryDir="${build.dir}/lockfactorytest/native" />
+ <testLockFactory lockFactoryImpl="org.apache.lucene.store.SimpleFSLockFactory" lockFactoryDir="${build.dir}/lockfactorytest/simple" />
</target>
<target name="test" depends="common.test, test-lock-factory"/>
Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java?rev=1589134&r1=1589133&r2=1589134&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java Tue Apr 22 14:17:40 2014
@@ -123,14 +123,10 @@ class NativeFSLock extends Lock {
path = new File(lockDir, lockFileName);
}
- private synchronized boolean lockExists() {
- return lock != null;
- }
-
@Override
public synchronized boolean obtain() throws IOException {
- if (lockExists()) {
+ if (lock != null) {
// Our instance is already locked:
return false;
}
@@ -150,7 +146,7 @@ class NativeFSLock extends Lock {
boolean success = false;
try {
lock = channel.tryLock();
- success = true;
+ success = lock != null;
} catch (IOException | OverlappingFileLockException e) {
// At least on OS X, we will sometimes get an
// intermittent "Permission Denied" IOException,
@@ -171,39 +167,20 @@ class NativeFSLock extends Lock {
}
}
}
- return lockExists();
+ return lock != null;
}
@Override
public synchronized void close() throws IOException {
- if (lockExists()) {
- try {
+ try {
+ if (lock != null) {
lock.release();
- } finally {
lock = null;
- try {
- channel.close();
- } finally {
- channel = null;
- }
}
- } 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) {
- close();
- }
+ } finally {
+ if (channel != null) {
+ channel.close();
+ channel = null;
}
}
}
@@ -213,7 +190,7 @@ class NativeFSLock extends Lock {
// The test for is isLocked is not directly possible with native file locks:
// First a shortcut, if a lock reference in this instance is available
- if (lockExists()) return true;
+ if (lock != null) return true;
// Look if lock file is present; if not, there can definitely be no lock!
if (!path.exists()) return false;
Modified: lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/store/TestLockFactory.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/store/TestLockFactory.java?rev=1589134&r1=1589133&r2=1589134&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/store/TestLockFactory.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/store/TestLockFactory.java Tue Apr 22 14:17:40 2014
@@ -214,25 +214,6 @@ public class TestLockFactory extends Luc
}
}
- public void testNativeFSLockReleaseByOtherLock() throws IOException {
- NativeFSLockFactory f = new NativeFSLockFactory(createTempDir(LuceneTestCase.getTestClass().getSimpleName()));
-
- 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.close();
- fail("should not have reached here. LockReleaseFailedException should have been thrown");
- } catch (LockReleaseFailedException e) {
- // expected
- } finally {
- l.close();
- }
- }
-
// Verify: NativeFSLockFactory assigns null as lockPrefix if the lockDir is inside directory
public void testNativeFSLockFactoryPrefix() throws IOException {