[368539] LoadRevisionsRequest with CDOFetchRuleManager enabled leads sometimes to NPE

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

3 files changed, 266 insertions, 10 deletions

diff --git a/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/LoadRevisionsIndication.java b/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/LoadRevisionsIndication.java
index 8668b3ac85..7501f8445c 100644
--- a/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/LoadRevisionsIndication.java
+++ b/plugins/org.eclipse.emf.cdo.server.net4j/src/org/eclipse/emf/cdo/server/internal/net4j/protocol/LoadRevisionsIndication.java
@@ -180,6 +180,7 @@ public class LoadRevisionsIndication extends CDOServerReadIndication
     }
 
     getRepository().notifyReadAccessHandlers(getSession(), revisions, additionalRevisions);
+
     for (int i = 0; i < size; i++)
     {
       RevisionInfo info = infos[i];
@@ -209,7 +210,13 @@ public class LoadRevisionsIndication extends CDOServerReadIndication
   private void collectRevisions(InternalCDORevision revision, Set<CDOID> revisions,
       List<CDORevision> additionalRevisions, Set<CDOFetchRule> visitedFetchRules)
   {
+    if (revision == null)
+    {
+      return;
+    }
+
     getSession().collectContainedRevisions(revision, branchPoint, referenceChunk, revisions, additionalRevisions);
+
     CDOFetchRule fetchRule = fetchRules.get(revision.getEClass());
     if (fetchRule == null || visitedFetchRules.contains(fetchRule))
     {
@@ -233,9 +240,12 @@ public class LoadRevisionsIndication extends CDOServerReadIndication
             if (!CDOIDUtil.isNull(id) && !revisions.contains(id))
             {
               InternalCDORevision containedRevision = getRevision(id);
-              revisions.add(containedRevision.getID());
-              additionalRevisions.add(containedRevision);
-              collectRevisions(containedRevision, revisions, additionalRevisions, visitedFetchRules);
+              if (containedRevision != null)
+              {
+                revisions.add(containedRevision.getID());
+                additionalRevisions.add(containedRevision);
+                collectRevisions(containedRevision, revisions, additionalRevisions, visitedFetchRules);
+              }
             }
           }
         }
@@ -249,9 +259,12 @@ public class LoadRevisionsIndication extends CDOServerReadIndication
           if (!id.isNull() && !revisions.contains(id))
           {
             InternalCDORevision containedRevision = getRevision(id);
-            revisions.add(containedRevision.getID());
-            additionalRevisions.add(containedRevision);
-            collectRevisions(containedRevision, revisions, additionalRevisions, visitedFetchRules);
+            if (containedRevision != null)
+            {
+              revisions.add(containedRevision.getID());
+              additionalRevisions.add(containedRevision);
+              collectRevisions(containedRevision, revisions, additionalRevisions, visitedFetchRules);
+            }
           }
         }
       }
@@ -328,8 +341,11 @@ public class LoadRevisionsIndication extends CDOServerReadIndication
     if (child == null)
     {
       child = getRevision(id);
-      map.put(id, child);
-      additionalRevisions.add(child);
+      if (child != null)
+      {
+        map.put(id, child);
+        additionalRevisions.add(child);
+      }
     }
 
     if (child != null && depth > 0)
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/FetchRuleAnalyzerTest.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/_FetchRuleAnalyzerTest_DISABLED_.java
index 522accd79f..7c27c9b7cf 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/FetchRuleAnalyzerTest.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/_FetchRuleAnalyzerTest_DISABLED_.java
@@ -35,7 +35,7 @@ import java.util.List;
  * 
  * @author Simon McDuff
  */
-public class FetchRuleAnalyzerTest extends AbstractCDOTest
+public class _FetchRuleAnalyzerTest_DISABLED_ extends AbstractCDOTest
 {
   public void testLoadObject() throws Exception
   {
@@ -95,7 +95,7 @@ public class FetchRuleAnalyzerTest extends AbstractCDOTest
     msg("Getting resource");
     for (CDOObject companyObject : listOfCompany)
     {
-      Company company = (Company)transaction.getObject(companyObject.cdoID(), true);
+      Company company = (Company)CDOUtil.getEObject(transaction.getObject(companyObject.cdoID(), true));
       for (PurchaseOrder purchaseOrder : company.getPurchaseOrders())
       {
         purchaseOrder.getSupplier();
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_368223_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_368223_Test.java
new file mode 100644
index 0000000000..7f3ba7a3d2
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_368223_Test.java
@@ -0,0 +1,240 @@
+/*

+ * Copyright (c) 2004 - 2011 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:

+ *    Ronald Krijgsheld - 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.config.IRepositoryConfig;

+import org.eclipse.emf.cdo.tests.config.impl.SessionConfig;

+import org.eclipse.emf.cdo.tests.model1.Category;

+import org.eclipse.emf.cdo.tests.model1.Company;

+import org.eclipse.emf.cdo.transaction.CDOTransaction;

+import org.eclipse.emf.cdo.util.CDOUtil;

+import org.eclipse.emf.cdo.util.InvalidObjectException;

+import org.eclipse.emf.cdo.util.ObjectNotFoundException;

+

+import org.eclipse.emf.common.util.EList;

+import org.eclipse.emf.ecore.EObject;

+

+import java.util.ArrayList;

+import java.util.List;

+import java.util.concurrent.atomic.AtomicReference;

+

+/**

+ * @author Ronald Krijgsheld

+ */

+public class Bugzilla_368223_Test extends AbstractCDOTest

+{

+  @Requires({ IRepositoryConfig.CAPABILITY_BRANCHING, "MEM" })

+  public void testRules1() throws Throwable

+  {

+    CDOSession session = openSession();

+

+    CDOTransaction transaction = session.openTransaction();

+    transaction.createResource(getResourcePath("/test1"));

+    transaction.commit();

+

+    AtomicReference<Throwable> exception = new AtomicReference<Throwable>();

+    long start = System.currentTimeMillis();

+

+    Creator creator = new Creator(exception);

+    creator.start();

+

+    Loader loader = new Loader(exception);

+    loader.start();

+

+    creator.join();

+    loader.join();

+

+    System.out.println(System.currentTimeMillis() - start);

+

+    Throwable ex = exception.get();

+    if (ex != null && !(ex instanceof Success))

+    {

+      throw exception.get();

+    }

+  }

+

+  @Override

+  protected void doSetUp() throws Exception

+  {

+    disableConsole();

+    super.doSetUp();

+  }

+

+  @Override

+  protected void doTearDown() throws Exception

+  {

+    disableConsole();

+    super.doTearDown();

+  }

+

+  /**

+   * @author Eike Stepper

+   */

+  private static final class Success extends Exception

+  {

+    private static final long serialVersionUID = 1L;

+  }

+

+  /**

+   * @author Eike Stepper

+   */

+  private abstract class Actor extends Thread

+  {

+    private final AtomicReference<Throwable> exception;

+

+    protected Actor(AtomicReference<Throwable> exception)

+    {

+      this.exception = exception;

+    }

+

+    @Override

+    public final void run()

+    {

+      try

+      {

+        runSafe(exception);

+      }

+      catch (Throwable ex)

+      {

+        exception.compareAndSet(null, ex);

+      }

+    }

+

+    protected abstract void runSafe(AtomicReference<Throwable> exception) throws Exception;

+  }

+

+  /**

+   * @author Ronald Krijgsheld

+   */

+  private final class Creator extends Actor

+  {

+    public Creator(AtomicReference<Throwable> exception)

+    {

+      super(exception);

+    }

+

+    @Override

+    protected void runSafe(AtomicReference<Throwable> exception) throws Exception

+    {

+      CDOSession session = openSession();

+      CDOTransaction transaction = session.openTransaction();

+      CDOResource resource = transaction.getResource(getResourcePath("/test1"));

+

+      List<Company> listOfCompanies = new ArrayList<Company>();

+

+      int loop = 10;

+      while (exception.get() == null && --loop != 0)

+      {

+        // System.out.println(loop);

+        synchronized (transaction)

+        {

+          for (int i = 0; i < 100; i++)

+          {

+            Company company = createCompanyWithCategories(resource);

+            listOfCompanies.add(company);

+          }

+

+          transaction.commit();

+

+          while (!listOfCompanies.isEmpty())

+          {

+            Company company = listOfCompanies.remove(0);

+

+            EList<Category> categories = company.getCategories();

+            while (!categories.isEmpty())

+            {

+              categories.remove(0);

+              transaction.commit();

+            }

+

+            resource.getContents().remove(company);

+            transaction.commit();

+          }

+        }

+      }

+

+      throw new Success();

+    }

+

+    private Company createCompanyWithCategories(CDOResource resource)

+    {

+      Company company = getModel1Factory().createCompany();

+      EList<Category> categories = company.getCategories();

+

+      for (int i = 0; i < 10; i++)

+      {

+        Category category = getModel1Factory().createCategory();

+        categories.add(category);

+      }

+

+      resource.getContents().add(company);

+      return company;

+    }

+  }

+

+  /**

+   * @author Ronald Krijgsheld

+   */

+  private final class Loader extends Actor

+  {

+    public Loader(AtomicReference<Throwable> exception)

+    {

+      super(exception);

+    }

+

+    @Override

+    protected void runSafe(AtomicReference<Throwable> exception) throws Exception

+    {

+      getTestProperties().put(SessionConfig.PROP_TEST_FETCH_RULE_MANAGER, CDOUtil.createThreadLocalFetchRuleManager());

+      CDOSession session = openSession();

+

+      CDOTransaction transaction = session.openTransaction();

+      transaction.options().setFeatureAnalyzer(CDOUtil.createModelBasedFeatureAnalyzer());

+

+      CDOResource resource = transaction.getResource(getResourcePath("/test1"));

+

+      while (exception.get() == null)

+      {

+        try

+        {

+          synchronized (transaction)

+          {

+            for (EObject object : resource.getContents())

+            {

+              Company company = (Company)object;

+              EList<Category> categories = company.getCategories();

+              if (categories.size() > 0)

+              {

+                Category category = categories.get(0);

+                msg(category);

+              }

+            }

+          }

+        }

+        catch (InvalidObjectException ex)

+        {

+          System.err.println(ex.getMessage());

+          continue;

+        }

+        catch (ObjectNotFoundException ex)

+        {

+          System.err.println(ex.getMessage());

+          continue;

+        }

+      }

+

+      throw new Success();

+    }

+  }

+}