From 06b48945b1dc3e02a8607fbcf94a2197dc6877e9 Mon Sep 17 00:00:00 2001
From: kelven <kelven@ky.(none)>
Date: Mon, 3 Jan 2011 23:05:51 -0800
Subject: [PATCH] Use double check pattern to avoid holding two locks in
 GlobalLock.java while trying to perserve intern semantic of global lock

---
 utils/src/com/cloud/utils/db/DbUtil.java     |  1 +
 utils/src/com/cloud/utils/db/GlobalLock.java | 72 +++++++++++---------
 2 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/utils/src/com/cloud/utils/db/DbUtil.java b/utils/src/com/cloud/utils/db/DbUtil.java
index a7dc8f4c540..05bb579a49f 100755
--- a/utils/src/com/cloud/utils/db/DbUtil.java
+++ b/utils/src/com/cloud/utils/db/DbUtil.java
@@ -249,6 +249,7 @@ public class DbUtil {
         Connection conn = getConnectionForGlobalLocks(name, false);
         if(conn == null) {
             s_logger.error("Unable to acquire DB connection for global lock system");
+            assert(false);
         	return false;
         }
         
diff --git a/utils/src/com/cloud/utils/db/GlobalLock.java b/utils/src/com/cloud/utils/db/GlobalLock.java
index 789da2deb25..c87106ef80d 100644
--- a/utils/src/com/cloud/utils/db/GlobalLock.java
+++ b/utils/src/com/cloud/utils/db/GlobalLock.java
@@ -67,18 +67,20 @@ public class GlobalLock {
 	public int releaseRef() {
 		int refCount;
 		
-		synchronized(s_lockMap) {	// // lock in sequence to prevent deadlock
-			synchronized(this) {
-				referenceCount--;
-				refCount = referenceCount;
-				
-				if(referenceCount < 0)
-					s_logger.warn("Unmatched Global lock " + name + " reference usage detected, check your code!");
-				
-				if(referenceCount == 0)
-					releaseInternLock(name);
-			}
-		}
+		boolean needToRemove = false;
+		synchronized(this) {
+			referenceCount--;
+			refCount = referenceCount;
+			
+			if(referenceCount < 0)
+				s_logger.warn("Unmatched Global lock " + name + " reference usage detected, check your code!");
+			
+			if(referenceCount == 0)
+				needToRemove = true;
+		}
+		
+		if(needToRemove)
+			releaseInternLock(name);
 		
 		return refCount;
 	}
@@ -99,8 +101,12 @@ public class GlobalLock {
 	}
 	
 	private static void releaseInternLock(String name) {
-		synchronized(s_lockMap) {
-			s_lockMap.remove(name);
+		synchronized(s_lockMap) {
+			GlobalLock lock = s_lockMap.get(name);
+			assert(lock != null);
+			
+			if(lock.referenceCount == 0)
+				s_lockMap.remove(name);
 		}
 	}
 	
@@ -135,25 +141,29 @@ public class GlobalLock {
 							return false;
 						
 						continue;
-					} else {
-						// we will discount the time that has been spent in previous waiting
+					} else {
+						// take ownership temporarily to prevent others enter into stage of acquiring DB lock
 						ownerThread = Thread.currentThread();
-						if(DbUtil.getGlobalLock(name, remainingMilliSeconds / 1000)) {
-							lockCount++;
-							holdingStartTick = System.currentTimeMillis();
-							
-							// keep the lock in the intern map when we got the lock from database
-							addRef();
-							
-							if(s_logger.isTraceEnabled())
-								s_logger.trace("lock " + name + " is acquired, lock count :" + lockCount);
-							return true;
-						} else {
-							ownerThread = null;
-						}
-						return false;
+						addRef();
 					}
-				}
+				}
+
+				if(DbUtil.getGlobalLock(name, remainingMilliSeconds / 1000)) {
+					synchronized(this) {
+						lockCount++;
+						holdingStartTick = System.currentTimeMillis();
+						
+						if(s_logger.isTraceEnabled())
+							s_logger.trace("lock " + name + " is acquired, lock count :" + lockCount);
+						return true;
+					}
+				} else {
+					synchronized(this) {
+						ownerThread = null;
+						releaseRef();
+						return false;
+					}
+				}
 			}
 		} finally {
 			if(interrupted) {