[359339] DeadLock in CDO Session drops/M20110930-0410

https://bugs.eclipse.org/bugs/show_bug.cgi?id=359339

3 files changed, 67 insertions, 57 deletions

diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_341995_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_341995_Test.java
index 97af85d0da..fee75ed3b8 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_341995_Test.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_341995_Test.java
@@ -22,12 +22,16 @@ import org.eclipse.emf.cdo.transaction.CDOTransaction;
 import org.eclipse.emf.cdo.util.CDOUtil;
 import org.eclipse.emf.cdo.util.CommitException;
 
+import java.util.concurrent.TimeUnit;
+
 /**
+ * Bug 341995.
+ * 
  * @author Caspar De Groot
  */
 public class Bugzilla_341995_Test extends AbstractCDOTest
 {
-  public void test() throws CommitException, InterruptedException
+  public void test() throws Exception
   {
     CDOSession session = openSession();
     CDOTransaction tx = session.openTransaction();
@@ -44,17 +48,18 @@ public class Bugzilla_341995_Test extends AbstractCDOTest
     long delay = 2000L;
     TestSessionManager sessionManager = (TestSessionManager)getRepository().getSessionManager();
     sessionManager.setCommitNotificationDelay(delay);
+
     try
     {
       doSecondSessionAsync();
-      sessionManager.getDelayLatch().await(); // Wait until the delay commences
+      sessionManager.getDelayLatch().await(DEFAULT_TIMEOUT, TimeUnit.MILLISECONDS); // Wait until the delay commences
 
       long time1 = System.currentTimeMillis();
 
       // Attempt the lock; this must block for a while, because it needs to receive
       // the commitNotification from the commit in the other session, which we are
       // artificially delaying
-      cdoCat.cdoWriteLock().lock();
+      cdoCat.cdoWriteLock().lock(DEFAULT_TIMEOUT, TimeUnit.MILLISECONDS);
 
       long timeTaken = System.currentTimeMillis() - time1;
 
@@ -83,6 +88,7 @@ public class Bugzilla_341995_Test extends AbstractCDOTest
         Category cat = (Category)resource.getContents().get(0);
         cat.setName("dirty");
         CDOCommitInfo info;
+
         try
         {
           info = tx.commit();
@@ -98,6 +104,9 @@ public class Bugzilla_341995_Test extends AbstractCDOTest
         session.close();
       }
     };
-    new Thread(r).start();
+
+    Thread thread = new Thread(r);
+    thread.setDaemon(true);
+    thread.start();
   }
 }
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/CDOSessionImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/CDOSessionImpl.java
index 31779a7b8b..d7806e0222 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/CDOSessionImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/CDOSessionImpl.java
@@ -761,32 +761,29 @@ public abstract class CDOSessionImpl extends Container<CDOView> implements Inter
 
   private long refresh(boolean enablePassiveUpdates)
   {
-    synchronized (outOfSequenceInvalidations)
-    {
-      Map<CDOBranch, List<InternalCDOView>> views = new HashMap<CDOBranch, List<InternalCDOView>>();
-      Map<CDOBranch, Map<CDOID, InternalCDORevision>> viewedRevisions = new HashMap<CDOBranch, Map<CDOID, InternalCDORevision>>();
-      collectViewedRevisions(views, viewedRevisions);
-      cleanupRevisionCache(viewedRevisions);
+    Map<CDOBranch, List<InternalCDOView>> views = new HashMap<CDOBranch, List<InternalCDOView>>();
+    Map<CDOBranch, Map<CDOID, InternalCDORevision>> viewedRevisions = new HashMap<CDOBranch, Map<CDOID, InternalCDORevision>>();
+    collectViewedRevisions(views, viewedRevisions);
+    cleanupRevisionCache(viewedRevisions);
 
-      CDOSessionProtocol sessionProtocol = getSessionProtocol();
-      long lastUpdateTime = getLastUpdateTime();
-      int initialChunkSize = options().getCollectionLoadingPolicy().getInitialChunkSize();
+    CDOSessionProtocol sessionProtocol = getSessionProtocol();
+    long lastUpdateTime = getLastUpdateTime();
+    int initialChunkSize = options().getCollectionLoadingPolicy().getInitialChunkSize();
 
-      RefreshSessionResult result = sessionProtocol.refresh(lastUpdateTime, viewedRevisions, initialChunkSize,
-          enablePassiveUpdates);
+    RefreshSessionResult result = sessionProtocol.refresh(lastUpdateTime, viewedRevisions, initialChunkSize,
+        enablePassiveUpdates);
 
-      setLastUpdateTime(result.getLastUpdateTime());
-      registerPackageUnits(result.getPackageUnits());
+    setLastUpdateTime(result.getLastUpdateTime());
+    registerPackageUnits(result.getPackageUnits());
 
-      for (Entry<CDOBranch, List<InternalCDOView>> entry : views.entrySet())
-      {
-        CDOBranch branch = entry.getKey();
-        List<InternalCDOView> branchViews = entry.getValue();
-        processRefreshSessionResult(result, branch, branchViews, viewedRevisions);
-      }
-
-      return result.getLastUpdateTime();
+    for (Entry<CDOBranch, List<InternalCDOView>> entry : views.entrySet())
+    {
+      CDOBranch branch = entry.getKey();
+      List<InternalCDOView> branchViews = entry.getValue();
+      processRefreshSessionResult(result, branch, branchViews, viewedRevisions);
     }
+
+    return result.getLastUpdateTime();
   }
 
   public void processRefreshSessionResult(RefreshSessionResult result, CDOBranch branch,
@@ -973,11 +970,8 @@ public abstract class CDOSessionImpl extends Container<CDOView> implements Inter
   {
     try
     {
-      synchronized (outOfSequenceInvalidations)
-      {
-        registerPackageUnits(commitInfo.getNewPackageUnits());
-        invalidate(commitInfo, null);
-      }
+      registerPackageUnits(commitInfo.getNewPackageUnits());
+      invalidate(commitInfo, null);
     }
     catch (RuntimeException ex)
     {
@@ -1177,38 +1171,41 @@ public abstract class CDOSessionImpl extends Container<CDOView> implements Inter
    */
   public void invalidate(CDOCommitInfo commitInfo, InternalCDOTransaction sender)
   {
-    synchronized (outOfSequenceInvalidations)
-    {
-      long previousTimeStamp = commitInfo.getPreviousTimeStamp();
-      long lastUpdateTime = getLastUpdateTime();
+    long previousTimeStamp = commitInfo.getPreviousTimeStamp();
+    long lastUpdateTime = getLastUpdateTime();
 
-      if (previousTimeStamp < lastUpdateTime)
-      {
-        previousTimeStamp = lastUpdateTime;
-      }
+    if (previousTimeStamp < lastUpdateTime)
+    {
+      previousTimeStamp = lastUpdateTime;
+    }
 
+    synchronized (outOfSequenceInvalidations)
+    {
       outOfSequenceInvalidations.put(previousTimeStamp, new Pair<CDOCommitInfo, InternalCDOTransaction>(commitInfo,
           sender));
+    }
 
-      long nextPreviousTimeStamp = lastUpdateTime;
-      while (!outOfSequenceInvalidations.isEmpty())
+    long nextPreviousTimeStamp = lastUpdateTime;
+    for (;;)
+    {
+      Pair<CDOCommitInfo, InternalCDOTransaction> currentPair;
+      synchronized (outOfSequenceInvalidations)
       {
-        Pair<CDOCommitInfo, InternalCDOTransaction> currentPair = outOfSequenceInvalidations
-            .remove(nextPreviousTimeStamp);
+        currentPair = outOfSequenceInvalidations.remove(nextPreviousTimeStamp);
+      }
 
-        // If we don't have the invalidation that follows the last one we processed,
-        // then there is nothing we can do right now
-        if (currentPair == null)
-        {
-          break;
-        }
+      // If we don't have the invalidation that follows the last one we processed,
+      // then there is nothing we can do right now
+      if (currentPair == null)
+      {
+        break;
+      }
 
-        CDOCommitInfo currentCommitInfo = currentPair.getElement1();
-        InternalCDOTransaction currentSender = currentPair.getElement2();
-        nextPreviousTimeStamp = currentCommitInfo.getTimeStamp();
+      CDOCommitInfo currentCommitInfo = currentPair.getElement1();
+      InternalCDOTransaction currentSender = currentPair.getElement2();
+      nextPreviousTimeStamp = currentCommitInfo.getTimeStamp();
 
-        invalidateOrdered(currentCommitInfo, currentSender);
-      }
+      invalidateOrdered(currentCommitInfo, currentSender);
     }
   }
 
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java
index ad78f2bfa6..7797140441 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java
@@ -256,6 +256,8 @@ public class CDOViewImpl extends AbstractCDOView
     checkActive();
     checkState(getTimeStamp() == CDOBranchPoint.UNSPECIFIED_DATE, "Locking not supported for historial views");
 
+    long endMillis = System.currentTimeMillis() + timeout;
+
     List<InternalCDORevision> revisions = new LinkedList<InternalCDORevision>();
     for (CDOObject object : objects)
     {
@@ -299,7 +301,8 @@ public class CDOViewImpl extends AbstractCDOView
       }
 
       long requiredTimestamp = result.getRequiredTimestamp();
-      waitForUpdate(requiredTimestamp);
+      long waitMillis = endMillis - System.currentTimeMillis();
+      waitForUpdate(requiredTimestamp, waitMillis);
     }
   }
 
@@ -837,9 +840,9 @@ public class CDOViewImpl extends AbstractCDOView
   public boolean waitForUpdate(long updateTime, long timeoutMillis)
   {
     long end = timeoutMillis == NO_TIMEOUT ? Long.MAX_VALUE : System.currentTimeMillis() + timeoutMillis;
-    for (;;)
+    synchronized (this)
     {
-      synchronized (this)
+      for (;;)
       {
         if (lastUpdateTime >= updateTime)
         {
@@ -854,7 +857,8 @@ public class CDOViewImpl extends AbstractCDOView
 
         try
         {
-          wait(end - now);
+          long waitMillis = end - now;
+          wait(waitMillis);
         }
         catch (InterruptedException ex)
         {