[jboss-svn-commits] JBL Code SVN: r26846 - labs/jbosstm/branches/JBOSSTS_4_6_1_GA_CP/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator.

[jboss-svn-commits at lists.jboss.org]

Author: adinn
Date: 2009-06-05 04:59:08 -0400 (Fri, 05 Jun 2009)
New Revision: 26846

Modified:
   labs/jbosstm/branches/JBOSSTS_4_6_1_GA_CP/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TransactionReaper.java
Log:
corrected errors in synchronization - fixes for JBTM-564

Modified: labs/jbosstm/branches/JBOSSTS_4_6_1_GA_CP/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TransactionReaper.java
===================================================================
--- labs/jbosstm/branches/JBOSSTS_4_6_1_GA_CP/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TransactionReaper.java	2009-06-05 08:48:04 UTC (rev 26845)
+++ labs/jbosstm/branches/JBOSSTS_4_6_1_GA_CP/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TransactionReaper.java	2009-06-05 08:59:08 UTC (rev 26846)
@@ -290,7 +290,7 @@
 			synchronized (_workQueue)
 			{
 			    _workQueue.add(e);
-			    _workQueue.notify();
+			    _workQueue.notifyAll();
 			}
 		    }
 		    break;
@@ -547,7 +547,7 @@
 		{
 		    e._worker = Thread.currentThread();
 		    e._status = ReaperElement.CANCEL;
-		    e.notify();
+		    e.notifyAll();
 		}
 
 		// we are now exposed to at most one interrupt from
@@ -624,14 +624,14 @@
 
 			cancelled = false;
 			e._status = ReaperElement.FAIL;
-			e.notify();
+			e.notifyAll();
 		    }
 		    else
 		    {
 			e._status = (cancelled
 				     ? ReaperElement.COMPLETE
 				     : ReaperElement.FAIL);
-			e.notify();
+			e.notifyAll();
 		    }
 		}
 
@@ -787,7 +787,7 @@
 
 			if(_dynamic)
 			{
-				notify(); // force recalc of next wakeup time, taking into account the newly inserted element
+				notifyAll(); // force recalc of next wakeup time, taking into account the newly inserted element
 			}
 
 			return rtn;
@@ -841,7 +841,7 @@
 
 		        return true;
 		    }
-                }
+        }
 	}
 
     /**
@@ -955,7 +955,9 @@
 
 	private final void shutdown (boolean waitForTransactions)
 	{
-	    synchronized (_shutdownLock)
+        // the reaper thread synchronizes and waits on this
+
+	    synchronized (this)
 	    {
 	        _inShutdown = true;
 
@@ -987,71 +989,64 @@
 	        {
 	            try
 	            {
-	                _shutdownLock.wait();
+	                this.wait();
 	            }
 	            catch (final Exception ex)
 	            {
 	            }
 	        }
 
-	        synchronized (_reaperThread)
-	        {
-	            _reaperThread.shutdown();
-	            _reaperThread.interrupt();  // by this stage there should be no transactions left anyway.
 
-	            synchronized (this)
-	            {
-	                notify();
-	            }
+            _reaperThread.shutdown();
 
-	            try
-	            {
-	                _reaperThread.join();
-	            }
-	            catch (final Exception ex)
-	            {
-	            }
-	        }
+            notifyAll();
+        }
+        try
+        {
+            _reaperThread.join();
+        }
+        catch (final Exception ex)
+        {
+        }
 
-	        _reaperThread = null;
+        _reaperThread = null;
 
-	        _reaperWorkerThread.shutdown();
+        // the reaper worker thread synchronizes and wais on the work queue
 
-	        synchronized (_reaperWorkerThread)
-	        {
-	            try
-	            {
-	                _reaperWorkerThread.interrupt();
-	                _reaperWorkerThread.join();
-	            }
-	            catch (final Exception ex)
-	            {
-	            }
-	        }
+        synchronized(_workQueue) {
+            _reaperWorkerThread.shutdown();
+            _workQueue.notifyAll();
+            // hmm, not sure we really need to do this but . . .
+            _reaperWorkerThread.interrupt();
+        }
 
-	        _reaperWorkerThread = null;
+        try
+        {
+            _reaperWorkerThread.join();
+        }
+        catch (final Exception ex)
+        {
+        }
 
-	        _inShutdown = false;
-	    }
+        _reaperWorkerThread = null;
 	}
 
 	/*
 	 * Remove element from list and trigger waiter if we are
 	 * being shutdown.
+	 *
+	 * n.b. must only be called when synchronized on this
 	 */
 
 	private final void removeElement (ReaperElement e)
 	{
-	    synchronized (_shutdownLock)
-	    {
-                _timeouts.remove(e._control);
-                _transactions.remove(e);
+        _timeouts.remove(e._control);
+        _transactions.remove(e);
 
-	        if (_inShutdown && (_transactions.size() == 0))
-	        {
-	            _shutdownLock.notify();
-	        }
-	    }
+        if (_inShutdown && (_transactions.size() == 0))
+        {
+            this.notifyAll();
+        }
 	}
 
 	/**
@@ -1116,6 +1111,10 @@
                         }
                     }
 				}
+                else
+                {
+                    checkPeriod = defaultCheckPeriod;
+                }
 			}
 			else
 				checkPeriod = Long.MAX_VALUE;
@@ -1256,7 +1255,7 @@
 	 * the transactions.
 	 */
 
-	public static void terminate (boolean waitForTransactions)
+	public static synchronized void terminate (boolean waitForTransactions)
 	{
 	    if (_theReaper != null)
 	    {
@@ -1327,10 +1326,5 @@
 
 	private static int _zombieCount = 0;
 
-	/*
-	 * Shutdown lock.
-	 */
-
-	private static final Object _shutdownLock = new Object();
-	private static boolean _inShutdown = false;
+	private boolean _inShutdown = false;
 }