[JGIT PATCH 06/13] Better handle concurrent reads during a WindowCache reconfiguration
- From
Shawn O. Pearce <spearce@spearce.org>
- Date
- Apr 28, 2009, 21:12 UTC
- Message-ID
- <1240953146-12878-7-git-send-email-spearce@spearce.org>
- In-Reply-To
- <1240953146-12878-6-git-send-email-spearce@spearce.org>
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 <spearce@spearce.org> --- .../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