Skip to content

Commit 5cfda39

Browse files
committed
Remove tryLockForReading/Writing methods from FileKeyLockManager
With respect to incorporating IoPolicy, the semantics of these methods are not clear. (Did we not get a lock because locking is broken for the file system, or because someone else already holds a write lock?) These methods were only used in tests. For simplicity, we just remove them for now.
1 parent 9501743 commit 5cfda39

3 files changed

Lines changed: 3 additions & 155 deletions

File tree

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

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -135,30 +135,6 @@ public LockedFileChannel lockForWriting(final Path path) throws IOException {
135135
return keyLockState(path).acquireWrite();
136136
}
137137

138-
/**
139-
* Attempts to acquire a read lock for the specified key without blocking.
140-
*
141-
* @param path
142-
* the file path to lock for reading
143-
* @return a {@link LockedChannel} if the lock was acquired, null otherwise
144-
*/
145-
public LockedFileChannel tryLockForReading(final Path path) {
146-
147-
return keyLockState(path).tryAcquireRead();
148-
}
149-
150-
/**
151-
* Attempts to acquire a write lock for the specified key without blocking.
152-
*
153-
* @param path
154-
* the file path to lock for writing
155-
* @return a {@link LockedChannel} if the lock was acquired, null otherwise
156-
*/
157-
public LockedFileChannel tryLockForWriting(final Path path) {
158-
159-
return keyLockState(path).tryAcquireWrite();
160-
}
161-
162138
/**
163139
* Returns the number of keys currently being tracked.
164140
*

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

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -85,56 +85,6 @@ LockedFileChannel acquireRead() throws IOException {
8585
}
8686
}
8787

88-
LockedFileChannel tryAcquireRead() {
89-
90-
if(!readerMutex.tryAcquire()) {
91-
return null;
92-
}
93-
94-
try {
95-
if (numReaders == 0) {
96-
// We are the first Reader, and are responsible for creating the channelLock
97-
// (Other concurrent Readers are still blocked in readerMutex.)
98-
99-
// If a Writer is still open, this will fail
100-
if(!channelLockMutex.tryAcquire()) {
101-
return null;
102-
}
103-
104-
try {
105-
channelLock = ChannelLock.tryLock(path, false);
106-
} catch (IOException e) {
107-
// Something went wrong. Back off.
108-
channelLockMutex.release();
109-
return null;
110-
}
111-
}
112-
113-
// We have a READ ChannelLock.
114-
// Try to open a FileChannel.
115-
final FileChannel channel;
116-
try {
117-
channel = FileChannel.open(path, StandardOpenOption.READ);
118-
} catch (final IOException e) {
119-
// Something went wrong. Back off.
120-
if (numReaders == 0) {
121-
try {
122-
releaseChannelLock();
123-
} catch (IOException ignored) {
124-
}
125-
}
126-
return null;
127-
}
128-
129-
// We have a FileChannel.
130-
// Create a LockedFileChannel that will releaseRead() when it is closed.
131-
++numReaders;
132-
return new LockedFileChannel(channel, this::releaseRead);
133-
} finally {
134-
readerMutex.release();
135-
}
136-
}
137-
13888
void releaseRead() throws IOException {
13989

14090
try {
@@ -184,25 +134,6 @@ LockedFileChannel acquireWrite() throws IOException {
184134
}
185135
}
186136

187-
LockedFileChannel tryAcquireWrite() {
188-
189-
if (!channelLockMutex.tryAcquire()) {
190-
return null;
191-
}
192-
193-
try {
194-
channelLock = ChannelLock.tryLock(path, true);
195-
} catch (IOException e) {
196-
// Something went wrong. Back off.
197-
channelLockMutex.release();
198-
return null;
199-
}
200-
201-
// We have a WRITE ChannelLock.
202-
// Create a LockedFileChannel that will releaseWrite() when it is closed.
203-
return new LockedFileChannel(channelLock.getChannel(), this::releaseWrite);
204-
}
205-
206137
void releaseWrite() throws IOException {
207138
releaseChannelLock();
208139
}

src/test/java/org/janelia/saalfeldlab/n5/FileKeyLockManagerTest.java

Lines changed: 3 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,15 @@
3333
import static org.junit.Assert.assertFalse;
3434
import static org.junit.Assert.assertNotNull;
3535
import static org.junit.Assert.assertNull;
36+
import static org.junit.Assert.assertThrows;
3637
import static org.junit.Assert.assertTrue;
3738

3839
import java.io.Closeable;
3940
import java.io.IOException;
4041
import java.io.Reader;
4142
import java.io.Writer;
4243
import java.nio.file.Files;
44+
import java.nio.file.NoSuchFileException;
4345
import java.nio.file.Path;
4446
import java.util.concurrent.CountDownLatch;
4547
import java.util.concurrent.ExecutorService;
@@ -194,66 +196,6 @@ public void testReadWriteExclusion() throws Exception {
194196
assertEquals("Reader should see written content", writtenContent, readContent.get());
195197
}
196198

197-
@Test
198-
public void testTryLock() throws Exception {
199-
200-
/* create a test file */
201-
final Path testFile = tempDir.resolve("test3.txt");
202-
final String testContent = "test content";
203-
Files.write(testFile, testContent.getBytes());
204-
205-
/* try acquiring read lock and read */
206-
LockedChannel readLock1 = FILE_LOCK_MANAGER.tryLockForReading(testFile);
207-
assertNotNull("Should acquire read lock", readLock1);
208-
try (final Reader reader = readLock1.newReader()) {
209-
final char[] buf = new char[testContent.length()];
210-
assertEquals("Should read correct number of chars", testContent.length(), reader.read(buf));
211-
}
212-
213-
/* try acquiring another read lock and read */
214-
LockedChannel readLock2 = FILE_LOCK_MANAGER.tryLockForReading(testFile);
215-
assertNotNull("Should acquire second read lock", readLock2);
216-
try (final Reader reader = readLock2.newReader()) {
217-
final char[] buf = new char[testContent.length()];
218-
assertEquals("Should read correct number of chars", testContent.length(), reader.read(buf));
219-
}
220-
221-
/* try acquiring write lock while reads are held - should fail */
222-
LockedChannel writeLock = FILE_LOCK_MANAGER.tryLockForWriting(testFile);
223-
assertNull("Should not acquire write lock while reads are held", writeLock);
224-
225-
readLock1.close();
226-
readLock2.close();
227-
228-
/* now try write lock and write */
229-
writeLock = FILE_LOCK_MANAGER.tryLockForWriting(testFile);
230-
assertNotNull("Should acquire write lock after reads are released", writeLock);
231-
final String newContent = "new content";
232-
try (final Writer writer = writeLock.newWriter()) {
233-
writer.write(newContent);
234-
}
235-
236-
/* try acquiring read lock from another thread while write is held */
237-
final AtomicReference<LockedChannel> readLock3 = new AtomicReference<>();
238-
final Thread readerThread = new Thread(() -> {
239-
readLock3.set(FILE_LOCK_MANAGER.tryLockForReading(testFile));
240-
});
241-
readerThread.start();
242-
readerThread.join();
243-
244-
assertNull("Should not acquire read lock while write is held", readLock3.get());
245-
246-
writeLock.close();
247-
248-
/* verify written content */
249-
try (final LockedChannel verifyLock = FILE_LOCK_MANAGER.lockForReading(testFile);
250-
final Reader reader = verifyLock.newReader()) {
251-
final char[] buf = new char[newContent.length()];
252-
reader.read(buf);
253-
assertEquals("Content should match what was written", newContent, new String(buf));
254-
}
255-
}
256-
257199
private class CleanUpHelper implements Closeable
258200
{
259201
private final Path path;
@@ -391,8 +333,7 @@ public void testWriteLockCreatesParentDirectories() throws Exception {
391333
@Test
392334
public void testReadLockRequiresExistingFile() throws Exception {
393335
final Path testFile = tempDir.resolve("nonexistent.txt");
394-
assertNull("Should not acquire read lock for non-existent file",
395-
FILE_LOCK_MANAGER.tryLockForReading(testFile));
336+
assertThrows("Should not acquire read lock for non-existent file", NoSuchFileException.class, () -> FILE_LOCK_MANAGER.lockForReading(testFile));
396337
}
397338

398339
@Test

0 commit comments

Comments
 (0)