thread safety, see Coverity issues 72313 and 72314

main/filesystem-inmemory/src/main/java/org/cryptomator/filesystem/inmem/InMemoryFile.java

@@ -13,6 +13,7 @@ import java.io.UncheckedIOException;
 import java.nio.ByteBuffer;
 import java.nio.file.FileAlreadyExistsException;
 import java.time.Instant;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
@@ -30,11 +31,16 @@ class InMemoryFile extends InMemoryNode implements File {
 	static final double GROWTH_RATE = 1.4;
 
 	private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
-	private volatile ByteBuffer content = ByteBuffer.allocate(INITIAL_SIZE);
+	private final AtomicReference<ByteBuffer> content = new AtomicReference<>(createNewEmptyByteBuffer());
 
 	public InMemoryFile(InMemoryFolder parent, String name, Instant lastModified, Instant creationTime) {
 		super(parent, name, lastModified, creationTime);
-		content.flip();
+	}
+
+	static ByteBuffer createNewEmptyByteBuffer() {
+		final ByteBuffer buf = ByteBuffer.allocate(INITIAL_SIZE);
+		buf.flip();
+		return buf;
 	}
 
 	@Override
@@ -47,9 +53,9 @@ class InMemoryFile extends InMemoryNode implements File {
 	}
 
 	private void internalMoveTo(InMemoryFile destination) {
-		this.content.rewind();
+		this.content.get().rewind();
 		destination.create();
-		destination.content = this.content;
+		destination.content.set(this.content.getAndSet(createNewEmptyByteBuffer()));
 		this.delete();
 	}
 
@@ -62,7 +68,7 @@ class InMemoryFile extends InMemoryNode implements File {
 		final ReadLock readLock = lock.readLock();
 		readLock.lock();
 		try {
-			final ReadableFile result = new InMemoryReadableFile(this::getContent, readLock);
+			final ReadableFile result = new InMemoryReadableFile(content::get, readLock);
 			success = true;
 			return result;
 		} finally {
@@ -79,7 +85,7 @@ class InMemoryFile extends InMemoryNode implements File {
 		writeLock.lock();
 		try {
 			create();
-			final WritableFile result = new InMemoryWritableFile(this::getContent, this::setContent, writeLock);
+			final WritableFile result = new InMemoryWritableFile(content::get, content::set, writeLock);
 			success = true;
 			return result;
 		} finally {
@@ -97,7 +103,7 @@ class InMemoryFile extends InMemoryNode implements File {
 				throw new UncheckedIOException(new FileAlreadyExistsException(k));
 			} else {
 				if (v == null) {
-					assert!content.hasRemaining();
+					assert!content.get().hasRemaining();
 					this.creationTime = Instant.now();
 				}
 				this.lastModified = Instant.now();
@@ -106,18 +112,9 @@ class InMemoryFile extends InMemoryNode implements File {
 		});
 	}
 
-	private ByteBuffer getContent() {
-		return content;
-	}
-
-	private void setContent(ByteBuffer content) {
-		this.content = content;
-	}
-
 	@Override
 	public void delete() {
-		content = ByteBuffer.allocate(INITIAL_SIZE);
-		content.flip();
+		content.set(createNewEmptyByteBuffer());
 		final InMemoryFolder parent = parent().get();
 		parent.existingChildren.computeIfPresent(this.name(), (k, v) -> {
 			// returning null removes the entry.

main/filesystem-inmemory/src/main/java/org/cryptomator/filesystem/inmem/InMemoryReadableFile.java

@@ -10,6 +10,8 @@ package org.cryptomator.filesystem.inmem;
 
 import java.io.UncheckedIOException;
 import java.nio.ByteBuffer;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock;
 import java.util.function.Supplier;
 
@@ -20,8 +22,8 @@ class InMemoryReadableFile implements ReadableFile {
 
 	private final Supplier<ByteBuffer> contentGetter;
 	private final ReadLock readLock;
-	private boolean open = true;
-	private volatile int position = 0;
+	private final AtomicInteger position = new AtomicInteger();
+	private final AtomicBoolean open = new AtomicBoolean(true);
 
 	public InMemoryReadableFile(Supplier<ByteBuffer> contentGetter, ReadLock readLock) {
 		this.contentGetter = contentGetter;
@@ -30,19 +32,21 @@ class InMemoryReadableFile implements ReadableFile {
 
 	@Override
 	public boolean isOpen() {
-		return open;
+		return open.get();
 	}
 
 	@Override
 	public int read(ByteBuffer destination) throws UncheckedIOException {
 		ByteBuffer source = contentGetter.get().asReadOnlyBuffer();
-		if (position >= source.limit()) {
+		int toBeCopied = destination.remaining();
+		int pos = position.getAndAdd(toBeCopied);
+		if (pos >= source.limit()) {
 			return -1;
 		} else {
-			source.position(position);
+			source.position(pos);
 			assert source.hasRemaining();
 			int numRead = ByteBuffers.copy(source, destination);
-			this.position += numRead;
+			assert numRead <= toBeCopied;
 			return numRead;
 		}
 	}
@@ -55,12 +59,12 @@ class InMemoryReadableFile implements ReadableFile {
 	@Override
 	public void position(long position) throws UncheckedIOException {
 		assert position < Integer.MAX_VALUE : "Can not use that big in-memory files.";
-		this.position = (int) position;
+		this.position.set((int) position);
 	}
 
 	@Override
 	public void close() throws UncheckedIOException {
-		open = false;
+		open.set(false);
 		readLock.unlock();
 	}
 

main/filesystem-inmemory/src/main/java/org/cryptomator/filesystem/inmem/InMemoryWritableFile.java

@@ -10,6 +10,9 @@ package org.cryptomator.filesystem.inmem;
 
 import java.io.UncheckedIOException;
 import java.nio.ByteBuffer;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
@@ -22,9 +25,9 @@ public class InMemoryWritableFile implements WritableFile {
 	private final Supplier<ByteBuffer> contentGetter;
 	private final Consumer<ByteBuffer> contentSetter;
 	private final WriteLock writeLock;
-
-	private boolean open = true;
-	private volatile int position = 0;
+	private final AtomicInteger position = new AtomicInteger();
+	private final AtomicBoolean open = new AtomicBoolean(true);
+	private final ReentrantLock writingLock = new ReentrantLock();
 
 	public InMemoryWritableFile(Supplier<ByteBuffer> contentGetter, Consumer<ByteBuffer> contentSetter, WriteLock writeLock) {
 		this.contentGetter = contentGetter;
@@ -34,44 +37,65 @@ public class InMemoryWritableFile implements WritableFile {
 
 	@Override
 	public boolean isOpen() {
-		return open;
+		return open.get();
 	}
 
 	@Override
 	public void truncate() throws UncheckedIOException {
-		contentSetter.accept(ByteBuffer.allocate(0));
+		writingLock.lock();
+		try {
+			contentSetter.accept(InMemoryFile.createNewEmptyByteBuffer());
+			position.set(0);
+		} finally {
+			writingLock.unlock();
+		}
 	}
 
 	@Override
 	public int write(ByteBuffer source) throws UncheckedIOException {
-		ByteBuffer destination = contentGetter.get();
-		int oldFileSize = destination.limit();
-		int requiredSize = position + source.remaining();
-		int newFileSize = Math.max(oldFileSize, requiredSize);
-		if (destination.capacity() < requiredSize) {
-			ByteBuffer old = destination;
-			old.clear();
-			int newBufferSize = Math.max(requiredSize, (int) (destination.capacity() * InMemoryFile.GROWTH_RATE));
-			destination = ByteBuffer.allocate(newBufferSize);
-			ByteBuffers.copy(old, destination);
-			contentSetter.accept(destination);
+		writingLock.lock();
+		try {
+			ByteBuffer content = contentGetter.get();
+			int prevLimit = content.limit();
+
+			int toBeCopied = source.remaining();
+			int pos = position.getAndAdd(toBeCopied);
+			int ourLimit = pos + toBeCopied;
+			int newLimit = Math.max(prevLimit, ourLimit);
+
+			ByteBuffer destination = ensureCapacity(content, newLimit);
+			destination.limit(newLimit).position(pos);
+			return ByteBuffers.copy(source, destination);
+		} finally {
+			writingLock.unlock();
+		}
+	}
+
+	private ByteBuffer ensureCapacity(ByteBuffer buf, int limit) {
+		assert writingLock.isHeldByCurrentThread();
+		if (buf.capacity() < limit) {
+			int oldPos = buf.position();
+			buf.clear();
+			int newBufferSize = Math.max(limit, (int) (buf.capacity() * InMemoryFile.GROWTH_RATE));
+			ByteBuffer newBuf = ByteBuffer.allocate(newBufferSize);
+			ByteBuffers.copy(buf, newBuf);
+			newBuf.limit(limit).position(oldPos);
+			contentSetter.accept(newBuf);
+			return newBuf;
+		} else {
+			return buf;
 		}
-		destination.limit(newFileSize);
-		destination.position(position);
-		int numWritten = ByteBuffers.copy(source, destination);
-		this.position += numWritten;
-		return numWritten;
 	}
 
 	@Override
 	public void position(long position) throws UncheckedIOException {
 		assert position < Integer.MAX_VALUE : "Can not use that big in-memory files.";
-		this.position = (int) position;
+		this.position.set((int) position);
 	}
 
 	@Override
 	public void close() throws UncheckedIOException {
-		open = false;
+		open.set(false);
 		writeLock.unlock();
 	}