From 9652f16a474371a387b125de600483735690f6b9 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 7 Nov 2011 12:06:56 -0800 Subject: [PATCH] Always use try/finally around DfsBlockCache.clockLock Any RuntimeException or Error in this block will leave the lock held by the caller thread, which can later result in deadlock or just cache requests hanging forever because they cannot get to the lock object. Wrap everything in try/finally to prevent the lock from hanging, even though a RuntimeException or Error should never happen in any of these code paths. Change-Id: Ibb3467f7ee4c06f617b737858b4be17b10d936e0 --- .../jgit/storage/dfs/DfsBlockCache.java | 76 ++++++++++--------- 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsBlockCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsBlockCache.java index e8733701d..ba704e510 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsBlockCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/dfs/DfsBlockCache.java @@ -389,36 +389,39 @@ public final class DfsBlockCache { @SuppressWarnings("unchecked") private void reserveSpace(int reserve) { clockLock.lock(); - long live = liveBytes + reserve; - if (maxBytes < live) { - Ref prev = clockHand; - Ref hand = clockHand.next; - do { - if (hand.hot) { - // Value was recently touched. Clear - // hot and give it another chance. - hand.hot = false; - prev = hand; + try { + long live = liveBytes + reserve; + if (maxBytes < live) { + Ref prev = clockHand; + Ref hand = clockHand.next; + do { + if (hand.hot) { + // Value was recently touched. Clear + // hot and give it another chance. + hand.hot = false; + prev = hand; + hand = hand.next; + continue; + } else if (prev == hand) + break; + + // No recent access since last scan, kill + // value and remove from clock. + Ref dead = hand; hand = hand.next; - continue; - } else if (prev == hand) - break; - - // No recent access since last scan, kill - // value and remove from clock. - Ref dead = hand; - hand = hand.next; - prev.next = hand; - dead.next = null; - dead.value = null; - live -= dead.size; - dead.pack.cachedSize.addAndGet(-dead.size); - statEvict++; - } while (maxBytes < live); - clockHand = prev; + prev.next = hand; + dead.next = null; + dead.value = null; + live -= dead.size; + dead.pack.cachedSize.addAndGet(-dead.size); + statEvict++; + } while (maxBytes < live); + clockHand = prev; + } + liveBytes = live; + } finally { + clockLock.unlock(); } - liveBytes = live; - clockLock.unlock(); } private void creditSpace(int credit) { @@ -429,13 +432,16 @@ public final class DfsBlockCache { private void addToClock(Ref ref, int credit) { clockLock.lock(); - if (credit != 0) - liveBytes -= credit; - Ref ptr = clockHand; - ref.next = ptr.next; - ptr.next = ref; - clockHand = ref; - clockLock.unlock(); + try { + if (credit != 0) + liveBytes -= credit; + Ref ptr = clockHand; + ref.next = ptr.next; + ptr.next = ref; + clockHand = ref; + } finally { + clockLock.unlock(); + } } void put(DfsBlock v) {