Attachment 128842 Details for Bug 266982 – To be reviewed

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] To be reviewed

clipboard.txt (text/plain), 15.81 KB, created by Simon Mc Duff

on 2009-03-15 10:42:12 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Mc Duff

Created: 2009-03-15 10:42:12 EDT

Size: 15.81 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.emf.cdo.tests
>Index: src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java
>===================================================================
>RCS file: /cvsroot/modeling/org.eclipse.emf/org.eclipse.emf.cdo/tests/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java,v
>retrieving revision 1.39
>diff -u -r1.39 AllTestsAllConfigs.java
>--- src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java	23 Feb 2009 09:14:53 -0000	1.39
>+++ src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java	15 Mar 2009 14:36:48 -0000
>@@ -41,6 +41,7 @@
> import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_260764_Test;
> import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_261218_Test;
> import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_265114_Test;
>+import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_266982_Test;
> import org.eclipse.emf.cdo.tests.config.impl.ConfigTest;
> import org.eclipse.emf.cdo.tests.config.impl.ConfigTestSuite;
> 
>@@ -128,6 +129,7 @@
>     testClasses.add(Bugzilla_260764_Test.class);
>     testClasses.add(Bugzilla_261218_Test.class);
>     testClasses.add(Bugzilla_265114_Test.class);
>+    testClasses.add(Bugzilla_266982_Test.class);
> 
>     // TODO testClasses.add(NonCDOResourceTest.class);
>     // TODO testClasses.add(GeneratedEcoreTest.class);
>Index: src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_266982_Test.java
>===================================================================
>RCS file: src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_266982_Test.java
>diff -N src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_266982_Test.java
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_266982_Test.java	1 Jan 1970 00:00:00 -0000
>@@ -0,0 +1,85 @@
>+/**
>+ * Copyright (c) 2004 - 2009 Eike Stepper (Berlin, Germany) and others.
>+ * All rights reserved. This program and the accompanying materials
>+ * are made available under the terms of the Eclipse Public License v1.0
>+ * which accompanies this distribution, and is available at
>+ * http://www.eclipse.org/legal/epl-v10.html
>+ * 
>+ * Contributors:
>+ *    Simon McDuff - initial API and implementation
>+ */
>+package org.eclipse.emf.cdo.tests.bugzilla;
>+
>+import org.eclipse.emf.cdo.eresource.CDOResource;
>+import org.eclipse.emf.cdo.session.CDOSession;
>+import org.eclipse.emf.cdo.tests.AbstractCDOTest;
>+import org.eclipse.emf.cdo.tests.model1.Customer;
>+import org.eclipse.emf.cdo.transaction.CDOTransaction;
>+import org.eclipse.emf.cdo.util.CDOUtil;
>+
>+import junit.framework.Assert;
>+
>+/**
>+ * IllegalStateException in CDOStore.getRevision
>+ * <p>
>+ * See https://bugs.eclipse.org/266982
>+ * 
>+ * @author Simon McDuff
>+ */
>+public class Bugzilla_266982_Test extends AbstractCDOTest
>+{
>+  public void testBugzilla_266982() throws Exception
>+  {
>+    final Customer customer = getModel1Factory().createCustomer();
>+    final boolean done[] = new boolean[1];
>+    final Exception exception[] = new Exception[1];
>+    done[0] = false;
>+    customer.setName("customer");
>+
>+    CDOSession session = openModel1Session();
>+    CDOTransaction transaction = session.openTransaction();
>+    CDOResource resource = transaction.createResource("/test1");
>+    resource.getContents().add(customer);
>+
>+    transaction.commit();
>+
>+    Runnable changeObjects = new Runnable()
>+    {
>+
>+      public void run()
>+      {
>+        try
>+        {
>+          CDOSession session = openModel1Session();
>+          CDOTransaction transaction = session.openTransaction();
>+          Customer customerToLoad = (Customer)transaction.getObject(CDOUtil.getCDOObject(customer).cdoID());
>+          while (!done[0])
>+          {
>+            // Could fail if the attach is not thread safe
>+            customerToLoad.getName();
>+          }
>+          transaction.close();
>+          session.close();
>+        }
>+        catch (Exception ex)
>+        {
>+          exception[0] = ex;
>+        }
>+      }
>+    };
>+    new Thread(changeObjects).start();
>+
>+    for (int i = 0; i < 500 && exception[0] == null; i++)
>+    {
>+      customer.setName("Ottawa" + i);
>+      transaction.commit();
>+    }
>+    done[0] = true;
>+    if (exception[0] != null)
>+    {
>+      exception[0].printStackTrace();
>+      Assert.fail(exception[0].getMessage());
>+    }
>+    session.close();
>+  }
>+}
>#P org.eclipse.emf.cdo
>Index: src/org/eclipse/emf/internal/cdo/CDOStateMachine.java
>===================================================================
>RCS file: /cvsroot/modeling/org.eclipse.emf/org.eclipse.emf.cdo/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOStateMachine.java,v
>retrieving revision 1.108
>diff -u -r1.108 CDOStateMachine.java
>--- src/org/eclipse/emf/internal/cdo/CDOStateMachine.java	14 Mar 2009 11:07:29 -0000	1.108
>+++ src/org/eclipse/emf/internal/cdo/CDOStateMachine.java	15 Mar 2009 14:36:49 -0000
>@@ -177,13 +177,21 @@
>    */
>   public void attach(InternalCDOObject object, InternalCDOTransaction transaction)
>   {
>-    List<InternalCDOObject> contents = new ArrayList<InternalCDOObject>();
>-    attach1(object, new Pair<InternalCDOTransaction, List<InternalCDOObject>>(transaction, contents));
>-    attach2(object);
>+    try
>+    {
>+      transaction.getStateLock().lock();
>+      List<InternalCDOObject> contents = new ArrayList<InternalCDOObject>();
>+      attach1(object, new Pair<InternalCDOTransaction, List<InternalCDOObject>>(transaction, contents));
>+      attach2(object);
> 
>-    for (InternalCDOObject content : contents)
>+      for (InternalCDOObject content : contents)
>+      {
>+        attach2(content);
>+      }
>+    }
>+    finally
>     {
>-      attach2(content);
>+      transaction.getStateLock().unlock();
>     }
>   }
> 
>@@ -219,35 +227,46 @@
>    */
>   public void detach(InternalCDOObject object)
>   {
>-    if (TRACER.isEnabled())
>+    InternalCDOView view = object.cdoView();
>+
>+    try
>     {
>-      trace(object, CDOEvent.DETACH);
>-    }
>+      view.getStateLock().lock();
>+
>+      if (TRACER.isEnabled())
>+      {
>+        trace(object, CDOEvent.DETACH);
>+      }
> 
>-    List<InternalCDOObject> objectsToDetach = new ArrayList<InternalCDOObject>();
>-    InternalCDOTransaction transaction = (InternalCDOTransaction)object.cdoView();
>+      List<InternalCDOObject> objectsToDetach = new ArrayList<InternalCDOObject>();
>+      InternalCDOTransaction transaction = (InternalCDOTransaction)object.cdoView();
> 
>-    // Accumulate objects that needs to be detached
>-    // If we have an error, we will keep the graph exactly like it was before.
>-    process(object, CDOEvent.DETACH, objectsToDetach);
>+      // Accumulate objects that needs to be detached
>+      // If we have an error, we will keep the graph exactly like it was before.
>+      process(object, CDOEvent.DETACH, objectsToDetach);
> 
>-    // postDetach requires the object to be TRANSIENT
>-    for (InternalCDOObject content : objectsToDetach)
>-    {
>-      CDOState oldState = content.cdoInternalSetState(CDOState.TRANSIENT);
>-      content.cdoInternalPostDetach();
>-      content.cdoInternalSetState(oldState);
>-    }
>+      // postDetach requires the object to be TRANSIENT
>+      for (InternalCDOObject content : objectsToDetach)
>+      {
>+        CDOState oldState = content.cdoInternalSetState(CDOState.TRANSIENT);
>+        content.cdoInternalPostDetach();
>+        content.cdoInternalSetState(oldState);
>+      }
> 
>-    // detachObject needs to know the state before we change the object to TRANSIENT
>-    for (InternalCDOObject content : objectsToDetach)
>-    {
>-      transaction.detachObject(content);
>-      content.cdoInternalSetState(CDOState.TRANSIENT);
>+      // detachObject needs to know the state before we change the object to TRANSIENT
>+      for (InternalCDOObject content : objectsToDetach)
>+      {
>+        transaction.detachObject(content);
>+        content.cdoInternalSetState(CDOState.TRANSIENT);
> 
>-      content.cdoInternalSetView(null);
>-      content.cdoInternalSetID(null);
>-      content.cdoInternalSetRevision(null);
>+        content.cdoInternalSetView(null);
>+        content.cdoInternalSetID(null);
>+        content.cdoInternalSetRevision(null);
>+      }
>+    }
>+    finally
>+    {
>+      view.getStateLock().unlock();
>     }
>   }
> 
>@@ -256,12 +275,22 @@
>    */
>   public void read(InternalCDOObject object)
>   {
>-    if (TRACER.isEnabled())
>+    InternalCDOView view = object.cdoView();
>+
>+    try
>     {
>-      trace(object, CDOEvent.READ);
>-    }
>+      view.getStateLock().lock();
>+      if (TRACER.isEnabled())
>+      {
>+        trace(object, CDOEvent.READ);
>+      }
> 
>-    process(object, CDOEvent.READ, null);
>+      process(object, CDOEvent.READ, null);
>+    }
>+    finally
>+    {
>+      view.getStateLock().unlock();
>+    }
>   }
> 
>   /**
>@@ -277,12 +306,22 @@
>    */
>   public void write(InternalCDOObject object, CDOFeatureDelta featureDelta)
>   {
>-    if (TRACER.isEnabled())
>+    InternalCDOView view = object.cdoView();
>+
>+    try
>     {
>-      trace(object, CDOEvent.WRITE);
>-    }
>+      view.getStateLock().lock();
>+      if (TRACER.isEnabled())
>+      {
>+        trace(object, CDOEvent.WRITE);
>+      }
> 
>-    process(object, CDOEvent.WRITE, featureDelta);
>+      process(object, CDOEvent.WRITE, featureDelta);
>+    }
>+    finally
>+    {
>+      view.getStateLock().unlock();
>+    }
>   }
> 
>   /**
>@@ -321,21 +360,30 @@
> 
>     if (view != null)
>     {
>-      InternalCDOSession session = (InternalCDOSession)view.getSession();
>-      revisions = session.getSessionProtocol().verifyRevision(revisions);
>-
>-      revisions.addAll(revised);
>-      for (InternalCDORevision revision : revisions)
>+      try
>       {
>-        InternalCDOObject object = ids.get(revision.getID());
>-        if (TRACER.isEnabled())
>+        ((InternalCDOView)view).getStateLock().lock();
>+        InternalCDOSession session = (InternalCDOSession)view.getSession();
>+        revisions = session.getSessionProtocol().verifyRevision(revisions);
>+
>+        revisions.addAll(revised);
>+        for (InternalCDORevision revision : revisions)
>         {
>-          trace(object, CDOEvent.RELOAD);
>-        }
>+          InternalCDOObject object = ids.get(revision.getID());
>+          if (TRACER.isEnabled())
>+          {
>+            trace(object, CDOEvent.RELOAD);
>+          }
> 
>-        process(object, CDOEvent.RELOAD, null);
>+          process(object, CDOEvent.RELOAD, null);
>+        }
>+      }
>+      finally
>+      {
>+        ((InternalCDOView)view).getStateLock().unlock();
>       }
>     }
>+
>   }
> 
>   /**
>@@ -343,12 +391,22 @@
>    */
>   public void invalidate(InternalCDOObject object, int version)
>   {
>-    if (TRACER.isEnabled())
>+    InternalCDOView view = object.cdoView();
>+
>+    try
>     {
>-      trace(object, CDOEvent.INVALIDATE);
>-    }
>+      view.getStateLock().lock();
>+      if (TRACER.isEnabled())
>+      {
>+        trace(object, CDOEvent.INVALIDATE);
>+      }
> 
>-    process(object, CDOEvent.INVALIDATE, version);
>+      process(object, CDOEvent.INVALIDATE, version);
>+    }
>+    finally
>+    {
>+      view.getStateLock().unlock();
>+    }
>   }
> 
>   /**
>@@ -356,12 +414,22 @@
>    */
>   public void detachRemote(InternalCDOObject object)
>   {
>-    if (TRACER.isEnabled())
>+    InternalCDOView view = object.cdoView();
>+
>+    try
>     {
>-      trace(object, CDOEvent.DETACH_REMOTE);
>-    }
>+      view.getStateLock().lock();
>+      if (TRACER.isEnabled())
>+      {
>+        trace(object, CDOEvent.DETACH_REMOTE);
>+      }
> 
>-    process(object, CDOEvent.DETACH_REMOTE, CDORevision.UNSPECIFIED_VERSION);
>+      process(object, CDOEvent.DETACH_REMOTE, CDORevision.UNSPECIFIED_VERSION);
>+    }
>+    finally
>+    {
>+      view.getStateLock().unlock();
>+    }
>   }
> 
>   /**
>@@ -369,12 +437,22 @@
>    */
>   public void commit(InternalCDOObject object, CommitTransactionResult result)
>   {
>-    if (TRACER.isEnabled())
>+    InternalCDOView view = object.cdoView();
>+
>+    try
>     {
>-      trace(object, CDOEvent.COMMIT);
>-    }
>+      view.getStateLock().lock();
>+      if (TRACER.isEnabled())
>+      {
>+        trace(object, CDOEvent.COMMIT);
>+      }
> 
>-    process(object, CDOEvent.COMMIT, result);
>+      process(object, CDOEvent.COMMIT, result);
>+    }
>+    finally
>+    {
>+      view.getStateLock().unlock();
>+    }
>   }
> 
>   /**
>@@ -382,12 +460,22 @@
>    */
>   public void rollback(InternalCDOObject object)
>   {
>-    if (TRACER.isEnabled())
>+    InternalCDOView view = object.cdoView();
>+
>+    try
>     {
>-      trace(object, CDOEvent.ROLLBACK);
>-    }
>+      view.getStateLock().lock();
>+      if (TRACER.isEnabled())
>+      {
>+        trace(object, CDOEvent.ROLLBACK);
>+      }
> 
>-    process(object, CDOEvent.ROLLBACK, null);
>+      process(object, CDOEvent.ROLLBACK, null);
>+    }
>+    finally
>+    {
>+      view.getStateLock().unlock();
>+    }
>   }
> 
>   @Override
>Index: src/org/eclipse/emf/internal/cdo/CDOStore.java
>===================================================================
>RCS file: /cvsroot/modeling/org.eclipse.emf/org.eclipse.emf.cdo/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOStore.java,v
>retrieving revision 1.80
>diff -u -r1.80 CDOStore.java
>--- src/org/eclipse/emf/internal/cdo/CDOStore.java	15 Mar 2009 10:59:04 -0000	1.80
>+++ src/org/eclipse/emf/internal/cdo/CDOStore.java	15 Mar 2009 14:36:50 -0000
>@@ -566,8 +566,16 @@
> 
>   private static InternalCDORevision getRevisionForReading(InternalCDOObject cdoObject)
>   {
>-    CDOStateMachine.INSTANCE.read(cdoObject);
>-    return getRevision(cdoObject);
>+    try
>+    {
>+      cdoObject.cdoView().getStateLock().lock();
>+      CDOStateMachine.INSTANCE.read(cdoObject);
>+      return getRevision(cdoObject);
>+    }
>+    finally
>+    {
>+      cdoObject.cdoView().getStateLock().unlock();
>+    }
>   }
> 
>   private static InternalCDORevision getRevisionForWriting(InternalCDOObject cdoObject, CDOFeatureDelta delta)
>Index: src/org/eclipse/emf/spi/cdo/InternalCDOView.java
>===================================================================
>RCS file: /cvsroot/modeling/org.eclipse.emf/org.eclipse.emf.cdo/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOView.java,v
>retrieving revision 1.3
>diff -u -r1.3 InternalCDOView.java
>--- src/org/eclipse/emf/spi/cdo/InternalCDOView.java	11 Jan 2009 09:25:07 -0000	1.3
>+++ src/org/eclipse/emf/spi/cdo/InternalCDOView.java	15 Mar 2009 14:36:51 -0000
>@@ -30,6 +30,7 @@
> 
> import java.util.Collection;
> import java.util.Set;
>+import java.util.concurrent.locks.ReentrantLock;
> 
> /**
>  * @author Eike Stepper
>@@ -98,4 +99,9 @@
>   public void unsubscribe(EObject eObject, Adapter adapter);
> 
>   public boolean hasSubscription(CDOID id);
>+
>+  /**
>+   * Used to attach and detach EObject with the corresponding CDORevision
>+   */
>+  public ReentrantLock getStateLock();
> }
>Index: src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java
>===================================================================
>RCS file: /cvsroot/modeling/org.eclipse.emf/org.eclipse.emf.cdo/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java,v
>retrieving revision 1.7
>diff -u -r1.7 CDOViewImpl.java
>--- src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java	14 Mar 2009 11:07:28 -0000	1.7
>+++ src/org/eclipse/emf/internal/cdo/view/CDOViewImpl.java	15 Mar 2009 14:36:51 -0000
>@@ -125,6 +125,8 @@
> 
>   private ReentrantLock lock = new ReentrantLock(true);
> 
>+  private ReentrantLock stateLock = new ReentrantLock(true);
>+
>   private CDOResourceImpl rootResource;
> 
>   private ChangeSubscriptionManager changeSubscriptionManager = createChangeSubscriptionManager();
>@@ -260,6 +262,14 @@
>   }
> 
>   /**
>+   * @since 2.0
>+   */
>+  public ReentrantLock getStateLock()
>+  {
>+    return lock;
>+  }
>+
>+  /**
>    * @throws InterruptedException
>    * @since 2.0
>    */

Actions: View | Diff

Attachments on bug 266982: 127443 | 128842 | 128848