Thread (18 messages) flat view 18 messages, 3 authors, 2016-06-15
DORMANTno replies

[JGIT PATCH 06/13] Better handle concurrent reads during a WindowCache reconfiguration

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:46:41
Subsystem: the rest · Maintainer: Linus Torvalds

If the WindowCache is reconfigured during a read we may wind up with
the newly read window referenced by the old cache.  This could cause
skew on the PackFile's reference counts, potentially never allowing
the pack's reference count to drop to 0, and pegging the file open.

One way to deal with this is to use a multi-reader/single-writer lock
around the cache variable, and only permit reconfiguration when there
are no concurrent accesses to the cache.  This is also quite costly
for an operation that occurs very infrequently, if ever.

Another approach is a double checked idiom.  If the cache was replaced
while we were accessing the window, there is a good chance that we put
the window back into the old cache.  If that occurs, calling removeAll
again on the old cache will ensure the window isn't referenced anymore
by that old cache.

Signed-off-by: Shawn O. Pearce <redacted>
---
 .../src/org/spearce/jgit/lib/WindowCache.java      |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
index aaad033..91c7cf3 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
@@ -123,7 +123,16 @@ public static void reconfigure(final WindowCacheConfig cfg) {
 	static final ByteWindow get(final PackFile pack, final long offset)
 			throws IOException {
 		final WindowCache c = cache;
-		return c.getOrLoad(pack, c.toStart(offset));
+		final ByteWindow r = c.getOrLoad(pack, c.toStart(offset));
+		if (c != cache) {
+			// The cache was reconfigured while we were using the old one
+			// to load this window. The window is still valid, but our
+			// cache may think its still live. Ensure the window is removed
+			// from the old cache so resources can be released.
+			//
+			c.removeAll();
+		}
+		return r;
 	}
 
 	static final void purge(final PackFile pack) {
-- 
1.6.3.rc1.205.g37f8
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help