From 1131f35cd1904d42fe86222c46212a3fb89b5da8 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Mon, 14 Jan 2019 10:48:06 -0800 Subject: [PATCH] dfs: Remove synchronization in BlockBasedFile#LazyChannel As explained in 'The "Double-Checked Locking is Broken" Declaration'[*], Java's memory model does not support double-checked locking: class LazyReadableChannel { private ReachableChannel rc = null; public ReadableChannel get() { if (rc == null) { synchronized (this) { if (rc == null) { rc = new ReadableChannel(); } } } return rc; } } With JDK 5 and newer, there is a formal memory model that ensures this works if "rc" is volatile, but it is still not thread-safe without that. Fortunately, this ReadableChannelSupplier is never passed between threads, so it does not need to be thread-safe. Simplify by removing the synchronization. [*] https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html Change-Id: I0698ee6618d734fc129dd4f63fc047c1c17c94a9 Signed-off-by: Jonathan Nieder --- .../eclipse/jgit/internal/storage/dfs/BlockBasedFile.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/BlockBasedFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/BlockBasedFile.java index 4d1474244..4e4a26f4e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/BlockBasedFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/BlockBasedFile.java @@ -213,7 +213,7 @@ abstract class BlockBasedFile { private final DfsPackDescription desc; private final PackExt ext; - private ReadableChannel rc = null; + private ReadableChannel rc; LazyChannel(DfsReader ctx, DfsPackDescription desc, PackExt ext) { this.ctx = ctx; @@ -224,11 +224,7 @@ abstract class BlockBasedFile { @Override public ReadableChannel get() throws IOException { if (rc == null) { - synchronized (this) { - if (rc == null) { - rc = ctx.db.openFile(desc, ext); - } - } + rc = ctx.db.openFile(desc, ext); } return rc; }