Revise IoPolicy

src/main/java/org/janelia/saalfeldlab/n5/BufferedKvaLockedChannel.java

@@ -1,9 +1,20 @@
 package org.janelia.saalfeldlab.n5;
 
+import java.io.BufferedWriter;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Reader;
+import java.io.Writer;
+import org.janelia.saalfeldlab.n5.N5Exception.N5IOException;
 import org.janelia.saalfeldlab.n5.readdata.ReadData;
 
-import java.io.*;
-
+/**
+ * Supports default implementation of the deprecated {@link KeyValueAccess#lockForReading(String)} and {@link KeyValueAccess#lockForWriting(String)} methods.
+ */
 class BufferedKvaLockedChannel implements LockedChannel {
 
     private final KeyValueAccess kva;
@@ -16,24 +27,27 @@ class BufferedKvaLockedChannel implements LockedChannel {
     }
 
     @Override
-    public Reader newReader() throws N5Exception.N5IOException {
+    public Reader newReader() throws N5IOException {
 
         return new InputStreamReader(newInputStream());
     }
 
     @Override
-    public InputStream newInputStream() throws N5Exception.N5IOException {
+    public InputStream newInputStream() throws N5IOException {
+
+		// TODO: This does not close the VolatileReadData returned by kva.createReadData.
+		//       Easiest fix is probably to materialize the VolatileReadData and close it, then use the materialized readData.
         return kva.createReadData(key).inputStream();
     }
 
     @Override
-    public Writer newWriter() throws N5Exception.N5IOException {
+    public Writer newWriter() throws N5IOException {
 
         return new BufferedWriter(new OutputStreamWriter(newOutputStream()));
     }
 
     @Override
-    public OutputStream newOutputStream() throws N5Exception.N5IOException {
+    public OutputStream newOutputStream() throws N5IOException {
         if (baos == null)
             baos = new ByteArrayOutputStream();
         return baos;

src/main/java/org/janelia/saalfeldlab/n5/ChannelLock.java

@@ -17,6 +17,18 @@
 class ChannelLock implements Closeable {
 
 	private final FileChannel channel;
+
+	/**
+	 * Hold a hard reference to the {@code FileLock} to make sure it is not
+	 * prematurely released.
+	 * <p>
+	 * NB: We do not call {@code lock.release()} in {@link #close}, because at
+	 * this point the channel might be already closed (by an external writer).
+	 * {@code lock.release()} will throw an exception if the channel is already
+	 * closed. Instead, we just close the channel which will automatically
+	 * release the lock.
+	 */
+	@SuppressWarnings({"unused", "FieldCanBeLocal"})
 	private final FileLock lock;
 
 	private ChannelLock(final FileChannel channel, final FileLock lock) {
@@ -53,9 +65,12 @@ FileChannel getChannel() {
 	 * @throws IOException if an error occurs while opening the channel, or if
 	 * the calling thread is interrupted while waiting for the {@code FileLock}.
 	 */
-	static ChannelLock lock(final Path path, final boolean forWriting) throws IOException {
+	static ChannelLock lock(final Path path, final boolean forWriting, final IoPolicy policy) throws IOException {
 
 		final FileChannel channel = openFileChannel(path, forWriting);
+		if (policy == IoPolicy.UNSAFE) {
+			return new ChannelLock(channel, null);
+		}
 		try {
 			while (true) {
 				try {
@@ -71,34 +86,12 @@ static ChannelLock lock(final Path path, final boolean forWriting) throws IOExce
 				}
 			}
 		} catch (Exception e) {
-			closeQuietly(channel);
-			throw e;
-		}
-	}
-
-	/**
-	 * Create a {@link FileChannel} on the given {@code path} and try to lock it
-	 * with a system-level {@link FileLock}. If the channel cannot be locked,
-	 * {@code null} is returned.
-	 * <p>
-	 * The {@code FileLock} is exclusive if the {@code path} is locked {@code
-	 * forWriting}, and shared otherwise.
-	 * <p>
-	 * If the {@code path} is locked {@code forWriting} non-existing file and
-	 * the parent directories are created as needed.
-	 *
-	 * @throws IOException if an error occurs while opening the channel.
-	 */
-	static ChannelLock tryLock(final Path path, final boolean forWriting) throws IOException {
-
-		FileChannel channel = null;
-		try {
-			channel = openFileChannel(path, forWriting);
-			final FileLock lock = channel.tryLock(0, Long.MAX_VALUE, !forWriting);
-			return lock == null ? null : new ChannelLock(channel, lock);
-		} catch (Exception e) {
-			closeQuietly(channel);
-			throw e;
+			if (policy == IoPolicy.STRICT) {
+				closeQuietly(channel);
+				throw e;
+			} else {
+				return new ChannelLock(channel, null);
+			}
 		}
 	}
 

src/main/java/org/janelia/saalfeldlab/n5/FileKeyLockManager.java

@@ -31,6 +31,7 @@
 import java.io.IOException;
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.WeakReference;
+import java.nio.channels.FileLock;
 import java.nio.file.Path;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -79,25 +80,26 @@ private void cleanUp()
 		}
 	}
 
-	private KeyLockState keyLockState(final Path path) {
+	private KeyLockState keyLockState(final Path path, final IoPolicy policy) throws IOException {
 
 		final String key = path.toAbsolutePath().toString();
 
 		cleanUp();
 
 		final WeakValue existingRef = locks.get(key);
 		KeyLockState state = existingRef == null ? null : existingRef.get();
-		if (state != null) {
-			return state;
+		if (state == null) {
+			final KeyLockState newState = new KeyLockState(path, policy);
+			while (state == null) {
+				final WeakValue ref = locks.compute(key,
+						(k, v) -> (v != null && v.get() != null)
+								? v
+								: new WeakValue(k, newState, refQueue));
+				state = ref.get();
+			}
 		}
-
-		final KeyLockState newState = new KeyLockState(path);
-		while (state == null) {
-			final WeakValue ref = locks.compute(key,
-					(k, v) -> (v != null && v.get() != null)
-							? v
-							: new WeakValue(k, newState, refQueue));
-			state = ref.get();
+		if (state.policy() != policy) {
+			throw new IOException("Trying to lock \"" + path + "\" with policy " + policy + ", but it is already used with " + state.policy());
 		}
 		return state;
 	}
@@ -108,55 +110,73 @@ private KeyLockState keyLockState(final Path path) {
 	 * <p>
 	 * The first reader will acquire a shared file lock. Subsequent readers
 	 * only acquire the thread-level lock.
+	 * <p>
+	 * The given locking {@link IoPolicy policy} applies to OS-level locking.
+	 * For both the {@code STRICT} and {@code PERMISSIVE} policy, a {@link
+	 * FileLock} is obtained. If this fails, {@code STRICT} will throw an {@code
+	 * IOException}. {@code PERMISSIVE} will proceed without locking. {@code
+	 * UNSAFE} will not attempt OS-level locking, however will still manage
+	 * mutual exclusion of readers and writers in the same JVM. Trying to lock
+	 * the same path with different locking policies will throw an {@code
+	 * IOException}.
 	 *
 	 * @param path
-	 *            the key (file path) to lock for reading
+	 * 		the key (file path) to lock for reading
+	 * @param policy
+	 * 		the locking policy
+	 *
 	 * @return a {@link LockedChannel} that must be closed when done
+	 *
 	 * @throws IOException
-	 *             if acquiring the file lock fails
+	 * 		if acquiring the file lock fails
 	 */
-	public LockedFileChannel lockForReading(final Path path) throws IOException {
+	public LockedFileChannel lockForReading(final Path path, final IoPolicy policy) throws IOException {
 
-		return keyLockState(path).acquireRead();
+		return keyLockState(path, policy).acquireRead();
 	}
 
 	/**
-	 * Acquires a write lock for the specified key. Only one thread can hold a
-	 * write lock for a key at a time, and no readers can hold locks.
-	 *
-	 * @param path
-	 *            the file path to lock for writing
-	 * @return a {@link LockedChannel} that must be closed when done
-	 * @throws IOException
-	 *             if acquiring the file lock fails
+	 * Acquires a read lock for the specified key with the {@link IoPolicy#STRICT} locking policy.
 	 */
-	public LockedFileChannel lockForWriting(final Path path) throws IOException {
+	public LockedFileChannel lockForReading(final Path path) throws IOException {
 
-		return keyLockState(path).acquireWrite();
+		return lockForReading(path, IoPolicy.STRICT);
 	}
 
 	/**
-	 * Attempts to acquire a read lock for the specified key without blocking.
+	 * Acquires a write lock for the specified key. Only one thread can hold a
+	 * write lock for a key at a time, and no readers can hold locks.
+	 * <p>
+	 * The given locking {@link IoPolicy policy} applies to OS-level locking.
+	 * For both the {@code STRICT} and {@code PERMISSIVE} policy, a {@link
+	 * FileLock} is obtained. If this fails, {@code STRICT} will throw an {@code
+	 * IOException}. {@code PERMISSIVE} will proceed without locking. {@code
+	 * UNSAFE} will not attempt OS-level locking, however will still manage
+	 * mutual exclusion of readers and writers in the same JVM. Trying to lock
+	 * the same path with different locking policies will throw an {@code
+	 * IOException}.
 	 *
 	 * @param path
-	 *            the file path to lock for reading
-	 * @return a {@link LockedChannel} if the lock was acquired, null otherwise
+	 * 		the file path to lock for writing
+	 * @param policy
+	 * 		the locking policy
+	 *
+	 * @return a {@link LockedChannel} that must be closed when done
+	 *
+	 * @throws IOException
+	 * 		if acquiring the file lock fails
 	 */
-	public LockedFileChannel tryLockForReading(final Path path) {
+	public LockedFileChannel lockForWriting(final Path path, final IoPolicy policy) throws IOException {
 
-		return keyLockState(path).tryAcquireRead();
+		return keyLockState(path, policy).acquireWrite();
 	}
 
 	/**
-	 * Attempts to acquire a write lock for the specified key without blocking.
-	 *
-	 * @param path
-	 *            the file path to lock for writing
-	 * @return a {@link LockedChannel} if the lock was acquired, null otherwise
+	 * Acquires a write lock for the specified key with the {@link IoPolicy#STRICT} locking policy.
 	 */
-	public LockedFileChannel tryLockForWriting(final Path path) {
+	public LockedFileChannel lockForWriting(final Path path) throws IOException {
 
-		return keyLockState(path).tryAcquireWrite();
+		return lockForWriting(path, IoPolicy.STRICT);
 	}
 
 	/**
@@ -168,14 +188,4 @@ int size() {
 
 		return locks.size();
 	}
-
-	/**
-	 * Removes the lock state for a key if no locks are held.
-	 *
-	 * @param key the key whose lock state should be removed
-	 * @return true if removed, false if currently in use or not found
-	 */
-	public boolean removeLockIfUnused(final String key) {
-		throw new UnsupportedOperationException("TODO. REMOVE");
-	}
 }