From 9124fcfaded66b5a4726f06d21cc98648a2004aa Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 26 May 2015 20:25:21 +0000 Subject: [PATCH] LUCENE-6499: WindowsFS misses to remove open file handle if file is concurrently deleted git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1681846 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/lucene/mockfile/WindowsFS.java | 32 +++++++--- .../apache/lucene/mockfile/TestWindowsFS.java | 63 +++++++++++++++++++ 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java b/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java index 1b732eb75f7..a4cfe2dde8e 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java +++ b/lucene/test-framework/src/java/org/apache/lucene/mockfile/WindowsFS.java @@ -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 openFiles = new HashMap<>(); - + final Map 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 HandleTrackingFS { @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 HandleTrackingFS { @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 HandleTrackingFS { @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); + } } } diff --git a/lucene/test-framework/src/test/org/apache/lucene/mockfile/TestWindowsFS.java b/lucene/test-framework/src/test/org/apache/lucene/mockfile/TestWindowsFS.java index 47e4311ccb1..10915da85ef 100644 --- a/lucene/test-framework/src/test/org/apache/lucene/mockfile/TestWindowsFS.java +++ b/lucene/test-framework/src/test/org/apache/lucene/mockfile/TestWindowsFS.java @@ -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 MockFileSystemTestCase { } is.close(); } + + public void testOpenDeleteConcurrently() throws IOException, Exception { + Path dir = wrap(createTempDir()); + 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 regeistered", 0, ((WindowsFS) dir.getFileSystem().provider()).openFiles.size()); + assertFalse("caught FNF on close", opened); + } + assertEquals("File handle leaked - file is closed but still regeistered", 0, ((WindowsFS) dir.getFileSystem().provider()).openFiles.size()); + Files.deleteIfExists(file); + } + } finally { + stopped.set(true); + t.join(); + } + } }