From c06e555510045959201d84b012b8b82352726b34 Mon Sep 17 00:00:00 2001 From: wcrisman Date: Sat, 12 Jul 2014 21:33:45 -0700 Subject: [PATCH] Added warnings to Monitor regarding unlocking with a thread that did not perform the locking; Added extra debug output for this occurrence. --- Common/src/com/common/thread/Monitor.java | 34 +++++++++++++++-------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/Common/src/com/common/thread/Monitor.java b/Common/src/com/common/thread/Monitor.java index 67ed8b5..362df2e 100644 --- a/Common/src/com/common/thread/Monitor.java +++ b/Common/src/com/common/thread/Monitor.java @@ -13,12 +13,15 @@ import com.common.util.optimized.IntArray; /** * This monitor allows protection of resources where waiting threads are given priority based on VM criteria. + *

Warning: This code REQUIRES that the locking thread also be the thread to unlock.

*/ public class Monitor { /** The thread local which maintains a hash map of lists of suspend and resumes for all monitors for the thread. The map is indexed by monitor, and the list is a stack of suspend counts. */ private static final ThreadLocal threadData = new ThreadLocal(); /** A flag to debug deadlocks. A deadlock is assumed after waiting 60 seconds for a lock without achiving it. If a deadlock is found then the locking thread's stack (at the time of obtaining the lock) is written to the debug stream in the form of a runtime exception. */ private static final boolean DEBUG = true; + /** Whether to log locking activity. */ + private static final boolean LOG_ACTIVITY = false; /** A monitor that will be locked if the calling thread asks to lock on a null monitor. */ private static final Monitor globalMonitor = new Monitor(); @@ -80,12 +83,14 @@ protected void setSupported(Object supported) { }//setSupported()// /** * Obtains a lock on the monitor, waiting indefinately for the the lock to become available. + *

Warning: This code REQUIRES that the locking thread also be the thread to unlock.

*/ public static void lock(Monitor monitor) { lock(monitor, 0, true, null); }//lock()// /** * Obtains a lock on the monitor. + *

Warning: This code REQUIRES that the locking thread also be the thread to unlock.

* @param timeout The number of milliseconds to wait before giving up. * @return Whether the lock was obtained. */ @@ -94,6 +99,7 @@ public static boolean lock(Monitor monitor, long timeout) { }//lock()// /** * Obtains a lock on the monitor. + *

Warning: This code REQUIRES that the locking thread also be the thread to unlock.

* @param monitor The monitor to be locked. * @param timeout The number of milliseconds to wait before giving up. * @param allowSuspend Whether the monitor should allow the requested lock to be suspended. If this is false and another lock is requested by the thread then an exception will be thrown. @@ -118,7 +124,7 @@ public static boolean lock(Monitor monitor, long timeout, boolean allowSuspend, }//if// if(threadData.currentLock == monitor) { - System.out.println("MONITOR: Incrementing lock count: " + Integer.toHexString(monitor.lock.hashCode())); + if(LOG_ACTIVITY) System.out.println("MONITOR: Incrementing lock count: " + Integer.toHexString(monitor.lock.hashCode())); threadData.lockCount++; threadData.stack = new RuntimeException(); @@ -143,7 +149,7 @@ public static boolean lock(Monitor monitor, long timeout, boolean allowSuspend, synchronized(monitor.lock) { if(monitor.lockingThreadData == null) { - System.out.println("MONITOR: Locking lock: " + Integer.toHexString(monitor.lock.hashCode())); + if(LOG_ACTIVITY) System.out.println("MONITOR: Locking lock: " + Integer.toHexString(monitor.lock.hashCode())); monitor.lockingThreadData = threadData; result = true; }//if// @@ -185,7 +191,7 @@ public static boolean lock(Monitor monitor, long timeout, boolean allowSuspend, //If we got the lock then proceed, otherwise we couldn't get the lock in 10 minutes of waiting... if(monitor.lockingThreadData == null) { - System.out.println("MONITOR: Locking lock: " + Integer.toHexString(monitor.lock.hashCode())); + if(LOG_ACTIVITY) System.out.println("MONITOR: Locking lock: " + Integer.toHexString(monitor.lock.hashCode())); monitor.lockingThreadData = threadData; result = true; }//if// @@ -206,7 +212,7 @@ public static boolean lock(Monitor monitor, long timeout, boolean allowSuspend, }//while// if(monitor.lockingThreadData == null) { - System.out.println("MONITOR: Locking lock: " + Integer.toHexString(monitor.lock.hashCode())); + if(LOG_ACTIVITY) System.out.println("MONITOR: Locking lock: " + Integer.toHexString(monitor.lock.hashCode())); monitor.lockingThreadData = threadData; result = true; }//if// @@ -246,6 +252,7 @@ public static boolean isLocked(Monitor monitor) { }//isLocked()// /** * Releases the lock and notifies threads waiting to get the lock. + *

Warning: This code REQUIRES that the locking thread also be the thread to unlock.

* @param monitor The monitor that is being unlocked (was previously locked). This is used to verify that the correct lock is being unlocked, and allows for early detection of bugs. */ public static void unlock(Monitor monitor) { @@ -257,11 +264,14 @@ public static void unlock(Monitor monitor) { }//if// if(threadData.currentLock == null) { - throw new SecurityException("Cannot unlock since the thread has no locks."); + //Note: Logging here because it seems that sometimes these exceptions disappear into a black hole.// + Debug.log(new SecurityException("Attempted unlock of a Monitor that was not locked at all.")); + throw new SecurityException(); }//if// else if(threadData.currentLock != monitor) { - Debug.log("Cannot unlock the monitor since it is not the currently locked monitor. The following stack (runtime-exception) shows the thread obtaining the currently held lock.", threadData.stack); - throw new SecurityException("Failed to unlock the monitor."); + //Note: Logging here because it seems that sometimes these exceptions disappear into a black hole.// + Debug.log(new SecurityException("Attempted unlock of a Monitor that was not locked by this thread.")); + throw new SecurityException(); }//else if// //Reduce the suspend prevention counter if necessary.// @@ -281,13 +291,13 @@ public static void unlock(Monitor monitor) { threadData.currentLock = null; threadData.lockCount = 0; threadData.stack = null; - System.out.println("MONITOR: Unlocking lock: " + Integer.toHexString(monitor.lock.hashCode())); + if(LOG_ACTIVITY) System.out.println("MONITOR: Unlocking lock: " + Integer.toHexString(monitor.lock.hashCode())); }//synchronized// resume(threadData); }//if// else { - System.out.println("MONITOR: Decremented lock count: " + Integer.toHexString(monitor.lock.hashCode())); + if(LOG_ACTIVITY) System.out.println("MONITOR: Decremented lock count: " + Integer.toHexString(monitor.lock.hashCode())); }//else// //}//if// }//unlock()// @@ -313,7 +323,7 @@ private static void suspend(ThreadData threadData) { if(threadData.currentLock != null) { //Allow another thread to lock the previously held monitor.// synchronized(threadData.currentLock.lock) { - System.out.println("MONITOR: Suspended lock: " + Integer.toHexString(threadData.currentLock.lock.hashCode())); + if(LOG_ACTIVITY) System.out.println("MONITOR: Suspended lock: " + Integer.toHexString(threadData.currentLock.lock.hashCode())); threadData.currentLock.lock.notify(); threadData.currentLock.lockingThreadData = null; threadData.lockCount = 0; @@ -358,7 +368,7 @@ private static void resume(ThreadData threadData) { //Wait until we can lock on the restored monitor.// synchronized(threadData.currentLock.lock) { if(threadData.currentLock.lockingThreadData == null) { - System.out.println("MONITOR: Resuming lock (no wait): " + Integer.toHexString(threadData.currentLock.lock.hashCode())); + if(LOG_ACTIVITY) System.out.println("MONITOR: Resuming lock (no wait): " + Integer.toHexString(threadData.currentLock.lock.hashCode())); threadData.currentLock.lockingThreadData = threadData; }//if// else { @@ -392,7 +402,7 @@ private static void resume(ThreadData threadData) { }//catch// }//while// - System.out.println("MONITOR: Resuming lock (after wait): " + Integer.toHexString(threadData.currentLock.lock.hashCode())); + if(LOG_ACTIVITY) System.out.println("MONITOR: Resuming lock (after wait): " + Integer.toHexString(threadData.currentLock.lock.hashCode())); threadData.currentLock.lockingThreadData = threadData; }//else// }//synchronized//