WIP multiple FileKeyLockManager instances

more or less copied from #211

Author: tpietzsch

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

@@ -2,7 +2,6 @@
 
 import java.io.Closeable;
 import java.io.IOException;
-import java.io.UncheckedIOException;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
 import java.nio.channels.OverlappingFileLockException;
@@ -18,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) {
@@ -54,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 LockingPolicy policy) throws IOException {
 
 		final FileChannel channel = openFileChannel(path, forWriting);
+		if (policy == LockingPolicy.UNSAFE) {
+			return new ChannelLock(channel, null);
+		}
 		try {
 			while (true) {
 				try {
@@ -72,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 == LockingPolicy.STRICT) {
+				closeQuietly(channel);
+				throw e;
+			} else {
+				return new ChannelLock(channel, null);
+			}
 		}
 	}
 
@@ -127,8 +119,8 @@ private static void closeQuietly(final FileChannel fileChannel) {
 		if (fileChannel != null) {
 			try {
 				fileChannel.close();
-			} catch (final IOException | UncheckedIOException ignored) {
+			} catch (final IOException ignored) {
 			}
-        }
+		}
 	}
 }

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

@@ -31,21 +31,44 @@
 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;
 
+import static org.janelia.saalfeldlab.n5.LockingPolicy.STRICT;
+
 /**
  * Provides thread-safe and process-safe read/write locking for filesystem paths.
  * Uses thread locks for JVM coordination and file locks for inter-process coordination.
  */
 class FileKeyLockManager {
 
-	static final FileKeyLockManager FILE_LOCK_MANAGER = new FileKeyLockManager();
+	/**
+	 * @deprecated use {@link FileKeyLockManager} instance per KVA instead of global one.
+	 */
+	@Deprecated
+	static final FileKeyLockManager FILE_LOCK_MANAGER = new FileKeyLockManager(STRICT);
 
-	private FileKeyLockManager() {
-		// singleton
-	}
+	private final LockingPolicy policy;
 
+	/**
+	 * Create a new {@link FileKeyLockManager} with the specified locking policy.
+	 * <p>
+	 * The given locking {@link LockingPolicy 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 policy
+	 * 		the locking policy
+	 */
+	FileKeyLockManager(final LockingPolicy policy) {
+		this.policy = policy;
+	}
 
 	private final ConcurrentHashMap<String, WeakValue> locks = new ConcurrentHashMap<>();
 
@@ -79,25 +102,26 @@ private void cleanUp()
 		}
 	}
 
-	private KeyLockState keyLockState(final Path path) {
+	private KeyLockState keyLockState(final Path path, final LockingPolicy 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;
 	}
@@ -110,29 +134,33 @@ private KeyLockState keyLockState(final Path path) {
 	 * only acquire the thread-level lock.
 	 *
 	 * @param path
-	 *            the key (file path) to lock for reading
+	 * 		the key (file path) to lock for reading
+	 *
 	 * @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 {
 
-		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
+	 * 		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
+	 * 		if acquiring the file lock fails
 	 */
 	public LockedFileChannel lockForWriting(final Path path) throws IOException {
 
-		return keyLockState(path).acquireWrite();
+		return keyLockState(path, policy).acquireWrite();
 	}
 
 	/**
@@ -144,14 +172,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");
-	}
 }

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

@@ -13,8 +13,11 @@ class KeyLockState {
 
 	private final Path path;
 
-	public KeyLockState(final Path path) {
+	private final LockingPolicy policy;
+
+	public KeyLockState(final Path path, LockingPolicy policy) {
 		this.path = path;
+		this.policy = policy;
 	}
 
 	/**
@@ -52,7 +55,7 @@ LockedFileChannel acquireRead() throws IOException {
 					channelLockMutex.acquire();
 
 					try {
-						channelLock = ChannelLock.lock(path, false);
+						channelLock = ChannelLock.lock(path, false, policy);
 					} catch (IOException e) {
 						// Something went wrong. Back off.
 						channelLockMutex.release();
@@ -119,7 +122,7 @@ LockedFileChannel acquireWrite() throws IOException {
 			channelLockMutex.acquire();
 
 			try {
-				channelLock = ChannelLock.lock(path, true);
+				channelLock = ChannelLock.lock(path, true, policy);
 			} catch (IOException e) {
 				// Something went wrong. Back off.
 				channelLockMutex.release();
@@ -137,4 +140,8 @@ LockedFileChannel acquireWrite() throws IOException {
 	void releaseWrite() throws IOException {
 		releaseChannelLock();
 	}
+
+	LockingPolicy policy() {
+		return policy;
+	}
 }

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

@@ -0,0 +1,54 @@
+package org.janelia.saalfeldlab.n5;
+
+/**
+ * File locking policy.
+ * <p>
+ * Usually, we want to coordinate reads and writs to a container such that
+ * <ul>
+ * <li>multiple readers can access a key simultaneously (blocking all writers).</li>
+ * <li>A writer should have exclusive access to a key (blocking all other readers and writers).</li>
+ * </ul>
+ * However, this cannot always be enforced for all backends:
+ * For SMB on macOS OS-level file locking is broken.
+ * For AWS S3 and Google Cloud we can detect (but not prevent) concurrent modifications.
+ * <p>
+ * Sometimes we know that we can disregard locking, for example in read-only settings.
+ * <p>
+ * {@code IoPolicy} can be used to configure locking policy for backends ({@link
+ * KeyValueAccess}) that support various locking strategies (potentially coming
+ * with performance trade-offs).
+ * <p>
+ * The policy values ({@link #STRICT}, {@link #UNSAFE}, {@link #PERMISSIVE})
+ * specify intent. Detailed interpretation is up to the backend implementation.
+ */
+public enum LockingPolicy {
+
+	/**
+	 * Protect all reads and writes by locks.
+	 * Fail if locking is not possible.
+	 */
+	STRICT,
+
+	/**
+	 * Reads and writes are unprotected.
+	 */
+	UNSAFE,
+
+	/**
+	 * Try to lock for all reads and writes.
+	 * Fall back to unprotected reads and writes if locking is not possible.
+	 * This is the default.
+	 */
+	PERMISSIVE;
+
+	static LockingPolicy fromString(final String s) {
+		if ("strict".equalsIgnoreCase(s))
+			return STRICT;
+		else if ("unsafe".equalsIgnoreCase(s))
+			return UNSAFE;
+//		else if ("permissive".equalsIgnoreCase(s))
+//			return PERMISSIVE;
+		else
+			return PERMISSIVE;
+	}
+}