You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by si...@apache.org on 2015/05/26 22:43:36 UTC
svn commit: r1681856 - in /lucene/dev/branches/lucene_solr_5_2: ./ lucene/
lucene/test-framework/
lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java
lucene/test-framework/src/test/org/apache/lucene/mockfile/TestWindowsFS.java
Author: simonw
Date: Tue May 26 20:43:35 2015
New Revision: 1681856
URL: http://svn.apache.org/r1681856
Log:
LUCENE-6499: WindowsFS misses to remove open file handle if file is concurrently deleted
Modified:
lucene/dev/branches/lucene_solr_5_2/ (props changed)
lucene/dev/branches/lucene_solr_5_2/lucene/ (props changed)
lucene/dev/branches/lucene_solr_5_2/lucene/test-framework/ (props changed)
lucene/dev/branches/lucene_solr_5_2/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java
lucene/dev/branches/lucene_solr_5_2/lucene/test-framework/src/test/org/apache/lucene/mockfile/TestWindowsFS.java
Modified: lucene/dev/branches/lucene_solr_5_2/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene_solr_5_2/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java?rev=1681856&r1=1681855&r2=1681856&view=diff
==============================================================================
--- lucene/dev/branches/lucene_solr_5_2/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java (original)
+++ lucene/dev/branches/lucene_solr_5_2/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java Tue May 26 20:43:35 2015
@@ -17,15 +17,19 @@ package org.apache.lucene.mockfile;
* limitations under the License.
*/
+import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.file.CopyOption;
import java.nio.file.FileSystem;
import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributeView;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
/**
* FileSystem that (imperfectly) acts like windows.
@@ -33,8 +37,7 @@ import java.util.Map;
* Currently this filesystem only prevents deletion of open files.
*/
public class WindowsFS extends HandleTrackingFS {
- private final Map<Object,Integer> openFiles = new HashMap<>();
-
+ final Map<Object,Integer> openFiles = new HashMap<>();
// TODO: try to make this as realistic as possible... it depends e.g. how you
// open files, if you map them, etc, if you can delete them (Uwe knows the rules)
@@ -60,8 +63,10 @@ public class WindowsFS extends HandleTra
@Override
protected void onOpen(Path path, Object stream) throws IOException {
- Object key = getKey(path);
synchronized (openFiles) {
+ final Object key = getKey(path);
+ // we have to read the key under the lock otherwise me might leak the openFile handle
+ // if we concurrently delete or move this file.
Integer v = openFiles.get(key);
if (v != null) {
v = Integer.valueOf(v.intValue()+1);
@@ -74,9 +79,10 @@ public class WindowsFS extends HandleTra
@Override
protected void onClose(Path path, Object stream) throws IOException {
- Object key = getKey(path);
+ Object key = getKey(path); // here we can read this outside of the lock
synchronized (openFiles) {
Integer v = openFiles.get(key);
+ assert v != null;
if (v != null) {
if (v.intValue() == 1) {
openFiles.remove(key);
@@ -111,19 +117,25 @@ public class WindowsFS extends HandleTra
@Override
public void delete(Path path) throws IOException {
- checkDeleteAccess(path);
- super.delete(path);
+ synchronized (openFiles) {
+ checkDeleteAccess(path);
+ super.delete(path);
+ }
}
@Override
public void move(Path source, Path target, CopyOption... options) throws IOException {
- checkDeleteAccess(source);
- super.move(source, target, options);
+ synchronized (openFiles) {
+ checkDeleteAccess(source);
+ super.move(source, target, options);
+ }
}
@Override
public boolean deleteIfExists(Path path) throws IOException {
- checkDeleteAccess(path);
- return super.deleteIfExists(path);
+ synchronized (openFiles) {
+ checkDeleteAccess(path);
+ return super.deleteIfExists(path);
+ }
}
}
Modified: lucene/dev/branches/lucene_solr_5_2/lucene/test-framework/src/test/org/apache/lucene/mockfile/TestWindowsFS.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene_solr_5_2/lucene/test-framework/src/test/org/apache/lucene/mockfile/TestWindowsFS.java?rev=1681856&r1=1681855&r2=1681856&view=diff
==============================================================================
--- lucene/dev/branches/lucene_solr_5_2/lucene/test-framework/src/test/org/apache/lucene/mockfile/TestWindowsFS.java (original)
+++ lucene/dev/branches/lucene_solr_5_2/lucene/test-framework/src/test/org/apache/lucene/mockfile/TestWindowsFS.java Tue May 26 20:43:35 2015
@@ -17,15 +17,25 @@ package org.apache.lucene.mockfile;
* limitations under the License.
*/
+import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.lang.Exception;
+import java.lang.InterruptedException;
+import java.lang.NoSuchFieldException;
+import java.lang.RuntimeException;
import java.net.URI;
import java.nio.file.FileSystem;
import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.lucene.mockfile.FilterPath;
+import org.apache.lucene.mockfile.WindowsFS;
import org.apache.lucene.util.Constants;
/** Basic tests for WindowsFS */
@@ -95,4 +105,57 @@ public class TestWindowsFS extends MockF
}
is.close();
}
+
+ public void testOpenDeleteConcurrently() throws IOException, Exception {
+ final Path dir = wrap(createTempDir());
+ final Path file = dir.resolve("thefile");
+ final CyclicBarrier barrier = new CyclicBarrier(2);
+ final AtomicBoolean stopped = new AtomicBoolean(false);
+ Thread t = new Thread() {
+ @Override
+ public void run() {
+ try {
+ barrier.await();
+ } catch (Exception ex) {
+ throw new RuntimeException(ex);
+ }
+ while (stopped.get() == false) {
+ try {
+ if (random().nextBoolean()) {
+ Files.delete(file);
+ } else if (random().nextBoolean()) {
+ Files.deleteIfExists(file);
+ } else {
+ Path target = file.resolveSibling("other");
+ Files.move(file, target);
+ Files.delete(target);
+ }
+ } catch (IOException ex) {
+ // continue
+ }
+ }
+ }
+ };
+ t.start();
+ barrier.await();
+ try {
+ final int iters = 10 + random().nextInt(100);
+ for (int i = 0; i < iters; i++) {
+ boolean opened = false;
+ try (OutputStream stream = Files.newOutputStream(file)) {
+ opened = true;
+ stream.write(0);
+ // just create
+ } catch (FileNotFoundException | NoSuchFileException ex) {
+ assertEquals("File handle leaked - file is closed but still registered", 0, ((WindowsFS) dir.getFileSystem().provider()).openFiles.size());
+ assertFalse("caught FNF on close", opened);
+ }
+ assertEquals("File handle leaked - file is closed but still registered", 0, ((WindowsFS) dir.getFileSystem().provider()).openFiles.size());
+ Files.deleteIfExists(file);
+ }
+ } finally {
+ stopped.set(true);
+ t.join();
+ }
+ }
}