[320976] ArrayIndexOutOfBoundsException in CDOListFeatureDeltaImpl

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

2 files changed, 66 insertions, 27 deletions

diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/delta/CDOListFeatureDeltaImpl.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/delta/CDOListFeatureDeltaImpl.java
index c636b6bb2b..a3fd03ee40 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/delta/CDOListFeatureDeltaImpl.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/delta/CDOListFeatureDeltaImpl.java
@@ -160,39 +160,39 @@ public class CDOListFeatureDeltaImpl extends CDOFeatureDeltaImpl implements CDOL
 
   private void reconstructAddedIndicesWithNoCopy()
   {
+    // Note that cachedIndices and cachedSources are always either both null or
+    // both non-null, and in the latter case, are always of the same length.
+    // Furthermore, there can only be unprocessedFeatureDeltas if cachesIndices
+    // and cachedSources are non-null.
+
     if (cachedIndices == null || unprocessedFeatureDeltas != null)
     {
-      List<CDOFeatureDelta> featureDeltasToBeProcessed = unprocessedFeatureDeltas == null ? featureDeltas
-          : unprocessedFeatureDeltas;
-
-      // Actually the required capacity is the number of ListTargetAdding instances in the
-      // featureDeltasToBeProcessed.. so this is an overestimate
-      int requiredCapacity = featureDeltasToBeProcessed.size() + 1;
-
       if (cachedIndices == null)
       {
-        cachedIndices = new int[1 + featureDeltas.size()];
+        int initialCapacity = featureDeltas.size() + 1;
+        cachedIndices = new int[initialCapacity];
+        cachedSources = new ListTargetAdding[initialCapacity];
       }
-      else if (cachedIndices.length < requiredCapacity)
+      else
+      // i.e. unprocessedFeatureDeltas != null
       {
-        int newCapacity = Math.max(requiredCapacity, cachedIndices.length * 3 / 2);
-        int[] newElements = new int[newCapacity];
-        System.arraycopy(cachedIndices, 0, newElements, 0, cachedIndices.length);
-        cachedIndices = newElements;
-      }
+        int requiredCapacity = 1 + cachedIndices[0] + unprocessedFeatureDeltas.size();
+        if (cachedIndices.length < requiredCapacity)
+        {
+          int newCapacity = Math.max(requiredCapacity, cachedIndices.length * 2);
 
-      if (cachedSources == null)
-      {
-        cachedSources = new ListTargetAdding[requiredCapacity];
-      }
-      else if (cachedSources.length <= requiredCapacity)
-      {
-        int newCapacity = Math.max(requiredCapacity, cachedSources.length * 3 / 2);
-        ListTargetAdding[] newElements = new ListTargetAdding[newCapacity];
-        System.arraycopy(cachedSources, 0, newElements, 0, cachedSources.length);
-        cachedSources = newElements;
+          int[] newIndices = new int[newCapacity];
+          System.arraycopy(cachedIndices, 0, newIndices, 0, cachedIndices.length);
+          cachedIndices = newIndices;
+
+          ListTargetAdding[] newSources = new ListTargetAdding[newCapacity];
+          System.arraycopy(cachedSources, 0, newSources, 0, cachedSources.length);
+          cachedSources = newSources;
+        }
       }
 
+      List<CDOFeatureDelta> featureDeltasToBeProcessed = unprocessedFeatureDeltas == null ? featureDeltas
+          : unprocessedFeatureDeltas;
       for (CDOFeatureDelta featureDelta : featureDeltasToBeProcessed)
       {
         if (featureDelta instanceof ListIndexAffecting)
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_302414_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_302414_Test.java
index 32fdcba5c1..4bcadac873 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_302414_Test.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_302414_Test.java
@@ -28,7 +28,7 @@ import java.util.List;
  */
 public class Bugzilla_302414_Test extends AbstractCDOTest
 {
-  public void test() throws CommitException
+  public void test1() throws CommitException
   {
     CDOSession session = openSession();
     CDOTransaction tx = session.openTransaction();
@@ -52,8 +52,47 @@ public class Bugzilla_302414_Test extends AbstractCDOTest
       company.getPurchaseOrders().add(Model1Factory.eINSTANCE.createPurchaseOrder());
     }
 
-    // The following statement must not fail!
-    list.remove(bar);
+    try
+    {
+      list.remove(bar);
+    }
+    catch (ArrayIndexOutOfBoundsException e)
+    {
+      e.printStackTrace();
+      fail("Should not have thrown " + e.getClass().getName());
+    }
+
+    tx.close();
+    session.close();
+  }
+
+  public void test2() throws CommitException
+  {
+    CDOSession session = openSession();
+    CDOTransaction tx = session.openTransaction();
+    CDOResource r1 = tx.createResource("/r1"); //$NON-NLS-1$
+
+    Company company = Model1Factory.eINSTANCE.createCompany();
+    r1.getContents().add(company);
+    tx.commit();
+
+    List<PurchaseOrder> list = company.getPurchaseOrders();
+
+    try
+    {
+      for (int i = 0; i < 20; i++)
+      {
+        PurchaseOrder foo1 = Model1Factory.eINSTANCE.createPurchaseOrder();
+        PurchaseOrder foo2 = Model1Factory.eINSTANCE.createPurchaseOrder();
+        list.add(foo1);
+        list.add(foo2);
+        list.remove(foo1);
+      }
+    }
+    catch (ArrayIndexOutOfBoundsException e)
+    {
+      fail("Should not have thrown " + e.getClass().getName());
+    }
 
     tx.close();
     session.close();