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

Bug 313326

Summary: NullPointerException in CDONotificationBuilder during Branch Merge
Product: [Modeling] EMF Reporter: Pascal Lehmann <pascal.lehmann>
Component: cdo.coreAssignee: Eike Stepper <stepper>
Status: CLOSED FIXED QA Contact: Eike Stepper <stepper>
Severity: normal    
Priority: P3 CC: cyril.jaquier, erwin, martin.fluegge, michael, stepper
Version: 3.0   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard: offline-12
Bug Depends on: 316594    
Bug Blocks:    
Attachments:
Description Flags
TestCase none

Description Pascal Lehmann CLA 2010-05-18 07:18:33 EDT 
Build Identifier: 3.0

The CDONotificationBuilder tries to provide the original Object as OldValue, therefore it uses the CDOID to look it up in the detached list.
In case of a remote-detach this works fine as the objects in the detached list are in state INVALID and still contain their ID/Revision.
In case of a branch merge the objects are detached normaly and end up in state TRANSIENT with their ID/Revision nulled, which then leads to the NullPointerException:

java.lang.NullPointerException
	at org.eclipse.emf.internal.cdo.CDONotificationBuilder.findDetachedObjectByID(CDONotificationBuilder.java:289)
	at org.eclipse.emf.internal.cdo.CDONotificationBuilder.visit(CDONotificationBuilder.java:151)
	at org.eclipse.emf.cdo.internal.common.revision.delta.CDOSetFeatureDeltaImpl.accept(CDOSetFeatureDeltaImpl.java:60)
	at org.eclipse.emf.cdo.internal.common.revision.delta.CDORevisionDeltaImpl.accept(CDORevisionDeltaImpl.java:249)
	at org.eclipse.emf.internal.cdo.CDONotificationBuilder.buildNotification(CDONotificationBuilder.java:95)
	at org.eclipse.emf.internal.cdo.view.CDOViewImpl.sendDeltaNotifications(CDOViewImpl.java:1532)
	at org.eclipse.emf.internal.cdo.transaction.CDOTransactionImpl.applyChangeSetData(CDOTransactionImpl.java:522)
	at org.eclipse.emf.internal.cdo.transaction.CDOTransactionImpl.merge(CDOTransactionImpl.java:358)

The exception can occur in all the visit-methods calling findDetachedObjectByID, however I never noticed it in visit(CDORemoveFeatureDelta) because the object was already found in the view and never had to call findDetachedObjectByID.
Adding view lookups to the visit-methods could help a bit, but the problem still exists in case the view doesn't know about the object.

Reproducible: Always

Steps to Reproduce:
1. Create 2 branches (Main and another one will do).
2. Detach an object on the other branch. (make sure eNotificationRequired() will be true on the container, eg. add an adapter)
3. Merge back to Main.
Comment 1 Eike Stepper CLA 2010-05-21 11:25:58 EDT 
Pascal, please provide a "simple" test case.
Comment 2 Pascal Lehmann CLA 2010-05-25 04:16:01 EDT 
Created attachment 169782 [details]
TestCase
Comment 3 Pascal Lehmann CLA 2010-05-25 04:22:17 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 4 Pascal Lehmann CLA 2010-05-31 05:23:08 EDT 
I noticed that there are NullPointerException problems with TRANSIENT objects at other points (eg. when trying to get that object from another view, where it is still contained and valid) because some of the fields are nulled.

Maybe some of the fields shouldn't be nulled? What is the exact purpose of nulling them?
Comment 5 Eike Stepper CLA 2010-05-31 05:30:05 EDT 
What fields exactly and where?
Comment 6 Pascal Lehmann CLA 2010-05-31 06:26:47 EDT 
cdoid, view and revision are nulled when an object goes into state TRANSIENT.
Comment 7 Eike Stepper CLA 2010-05-31 06:49:50 EDT 
I think "view" must be nulled. And the (former) revisions can be obtained via InternalCDOTransaction.getFormerRevisions().
Comment 8 Pascal Lehmann CLA 2010-05-31 07:05:25 EDT 
Ok, it probably does make sense to null those fields since the object isn't contained in that view anymore, and therefore doesn't have any valid revision and cdoid.
Comment 9 Eike Stepper CLA 2010-06-05 04:16:45 EDT 
Committed test case.

The test is passing, so I assume that one of our other fixes to invalidation handling and notifications has fixed this issue, as well. Resolving FIXED and please reopen if you don't agree.
Comment 10 Eike Stepper CLA 2010-06-05 07:12:22 EDT 
Reopening because I realized that the test does *not* pass on Hudson. At home I can not reproduce this issue. It's strange because I've had the very same effect with a different test case yesterday: ChangeSubscriptionTest.testInvalidationWithDeltas_SubBranch()

I need help to reproduce this in a debugging environment!
Comment 11 Eike Stepper CLA 2010-06-23 07:35:05 EDT 
Should be fixed with bug 316594...
Comment 12 Eike Stepper CLA 2010-06-29 04:54:27 EDT 
Rebasing all outstanding 3.0 problem reports to version 3.0.1
Comment 13 Martin Fluegge CLA 2010-07-06 12:27:53 EDT 
Hi guys,

I tried to reproduce the NPE several times but was neither und HEAD nor on R3_0_Maintenance successful. The test either passes or failes with the ObjectNotFoundException mentioned in Bug 316594.

Eike, maybe the Exception you mentioned in comment #10 was the ONFE (which always occurs on Hudson) and not the NPE?

Pascal, could you check whether you are still able to reproduce the exception. Maybe it is already fixed due to another fix, as Eike assumed before.

If we cannot reproduce this I would suggest closing this one as WORKSFORME.
Comment 14 Pascal Lehmann CLA 2010-07-07 02:42:04 EDT 
Hi Martin, Eike

Sorry for the confusion, I provided a fix for the NPE along with a patch for bug #306710 (comment9 and 10) and forgot to mention it here.

Since the NPE is fixed I guess this bug can be closed.
Comment 15 Eike Stepper CLA 2010-07-07 02:56:25 EDT 
Thank you guys.
Comment 16 Eike Stepper CLA 2010-07-07 02:57:35 EDT 
Available in 3.0 GA:
http://download.eclipse.org/modeling/emf/cdo/updates/3.0-releases/