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

Bug 306710

Summary:

IndexOutOfBoundsException upon invalidation

Product:

[Modeling] EMF

Reporter:

Victor Roldan Betancort <vroldanbet>

Component:

cdo.core

Assignee:

Eike Stepper <stepper>

Status:

CLOSED FIXED

QA Contact:

Eike Stepper <stepper>

Severity:

normal

Priority:

CC:

cyril.jaquier, erwin, michael, pascal.lehmann

Version:

3.0

Target Milestone:

---

Hardware:

OS:

Windows XP

Whiteboard:

offline-11

Bug Depends on:

310574

Bug Blocks:

Attachments:

Description	Flags
TestCase	none
Patch Proposal for the Notification Builder	none
better patch for CDONotificationBuilder	none
CDODeltaNotification patch proposal	none
Updated TestCase	none
Enable and fix the second test in the TestCase after the fix for bug 310574	none

Description Victor Roldan Betancort

2010-03-22 09:37:42 EDT

While testing invalidation and conflict notification in two different clients, I saw this in one of the clients:


[ERROR] Index: 1, Size: 1
java.lang.IndexOutOfBoundsException: Index: 1, Size: 1
	at java.util.ArrayList.RangeCheck(Unknown Source)
	at java.util.ArrayList.get(Unknown Source)
	at org.eclipse.emf.cdo.spi.common.revision.BaseCDORevision.get(BaseCDORevision.java:406)
	at org.eclipse.emf.internal.cdo.CDONotificationBuilder.visit(CDONotificationBuilder.java:122)
	at org.eclipse.emf.cdo.internal.common.revision.delta.CDORemoveFeatureDeltaImpl.accept(CDORemoveFeatureDeltaImpl.java:74)
	at org.eclipse.emf.internal.cdo.CDONotificationBuilder.visit(CDONotificationBuilder.java:160)
	at org.eclipse.emf.cdo.internal.common.revision.delta.CDOListFeatureDeltaImpl.accept(CDOListFeatureDeltaImpl.java:263)
	at org.eclipse.emf.cdo.internal.common.revision.delta.CDORevisionDeltaImpl.accept(CDORevisionDeltaImpl.java:249)
	at org.eclipse.emf.internal.cdo.CDONotificationBuilder.buildNotification(CDONotificationBuilder.java:91)
	at org.eclipse.emf.internal.cdo.view.CDOViewImpl.sendDeltaNotifications(CDOViewImpl.java:1511)
	at org.eclipse.emf.internal.cdo.view.CDOViewImpl.invalidate(CDOViewImpl.java:1360)
	at org.eclipse.emf.internal.cdo.session.CDOSessionImpl$InvalidationRunnable.run(CDOSessionImpl.java:1204)
	at org.eclipse.net4j.util.concurrent.QueueRunner.work(QueueRunner.java:26)
	at org.eclipse.net4j.util.concurrent.QueueRunner.work(QueueRunner.java:1)
	at org.eclipse.net4j.util.concurrent.QueueWorker.work(QueueWorker.java:75)
	at org.eclipse.net4j.util.concurrent.Worker$WorkerThread.run(Worker.java:188)

Comment 1 Pascal Lehmann

2010-04-15 08:17:00 EDT

This also happens for other CDOFeatureDeltas (eg. ADD).
The problem is that when the invalidation removes remote_detached objects from your dirty transaction, indexes for the RevisionDeltas are not adjusted.

Comment 2 Pascal Lehmann

2010-04-19 10:18:31 EDT

Created attachment 165282 [details]
TestCase

Comment 3 Pascal Lehmann

2010-04-19 10:19:16 EDT

forget my last comment, I was confusing the error with another problem with invalidation and collisions.

But I attached a testcase for your problem.

Comment 4 Michael Szediwy

2010-05-06 13:08:52 EDT

Created attachment 167345 [details]
Patch Proposal for the Notification Builder

Comment 5 Eike Stepper

2010-05-07 05:05:18 EDT

Hi Pascal and Michael,

Thanks for the test case and the patch! Can you (both) please confirm the following:

1) The number of lines that you added/changed is smaller than 250.
2) You are the only author of these changed lines.
3) You apply the EPL to these changed lines.

Comment 6 Eike Stepper

2010-05-07 05:06:43 EDT

Committed to HEAD

Comment 7 Pascal Lehmann

2010-05-07 06:00:06 EDT

1) The number of lines that you added/changed is smaller than 250.
confirmed
2) You are the only author of these changed lines.
confirmed
3) You apply the EPL to these changed lines.
confirmed

Comment 8 Michael Szediwy

2010-05-08 04:54:21 EDT

1) The number of lines that you added/changed is smaller than 250.
confirmed
2) You are the only author of these changed lines.
confirmed
3) You apply the EPL to these changed lines.
confirmed

Comment 9 Pascal Lehmann

2010-05-28 08:15:24 EDT

The originally attached patch does fix the IndexOutOfBounds problem, but introduces other problems later in the notification chain because of the patched indices (eg. DataBinding).
Here is a new patch which does not tamper with the indices of the notification itself, but uses patched indices internally to retrieve the values for the old objects.

Other changes introduced:
- NPE should be fixed: bug #313326
- Lookup for old Object is always done in the view first, before looking in the detached list.
- OldValue is now always CDOID or the old Object, was sometimes null before.

The last change lead to occasional ObjectNotFoundException while accessing the oldValue of the Notification, therefore I introduced also a patch for CDODeltaNotificationImpl.

Comment 10 Pascal Lehmann

2010-05-28 08:17:00 EDT

Created attachment 170330 [details]
better patch for CDONotificationBuilder

Comment 11 Pascal Lehmann

2010-05-28 08:18:03 EDT

Created attachment 170332 [details]
CDODeltaNotification patch proposal

1) The number of lines that you added/changed is smaller than 250.
confirmed
2) You are the only author of these changed lines.
confirmed
3) You apply the EPL to these changed lines.
confirmed

Comment 12 Pascal Lehmann

2010-06-01 09:19:14 EDT

please reopen.

Comment 13 Eike Stepper

2010-06-02 03:57:20 EDT

Reopened

Comment 14 Pascal Lehmann

2010-06-03 10:01:16 EDT

Created attachment 170958 [details]
Updated TestCase

I added another test which still fails, even with the newer patch.
The new problem is that the CDONotificationBuilder tries to get the detached object which was attached in the same commit (and therefore is in the same RevisionDelta). This of course can not work as the old revision only knows the values attached anytime before the last commit.

I'm unsure if this should be fixed with bug #310574 or if this should be handled here as well.

Comment 15 Eike Stepper

2010-06-04 04:10:13 EDT

I disabled _testBugzilla_306710_addRemove() for now. After fixing bug 310574 this must be revisited!!!

Committed to HEAD.

Comment 16 Cyril Jaquier

2010-06-11 07:54:21 EDT

Created attachment 171709 [details]
Enable and fix the second test in the TestCase after the fix for bug 310574

This patch enables again and fixes testBugzilla_306710_addRemove after the fix for bug 310574.

Comment 17 Eike Stepper

2010-06-23 07:34:03 EDT

Thank you Cyril!

Comment 18 Eike Stepper

2010-06-29 04:36:19 EDT

Available in 3.0 GA:
http://download.eclipse.org/modeling/emf/cdo/updates/3.0-releases/