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

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 e8733701df620e5e80e8a500b4bde2d63a1735af..ba704e510456b834e8a7bd72ed66063671f0bd49 100644 (file)
--- 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) {