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

Bug 325866

Summary: IllegalStateException when committing a reattached containment of a detached container after branch merge with XRef enabled
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: erwin
Version: 4.2   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard: offline-01
Attachments:
Description Flags
Testcase
stepper: iplog+
Proposed patch
stepper: iplog+
Combined patch - for future reference
none
Testcase v2 none

Description Pascal Lehmann CLA 2010-09-21 11:08:06 EDT 
Build Identifier: 4.0

We have a containment tree which is 3 levels deep. On the branch we detach level2 object and reattach the level3 child of the now detached level2 object to the children list of level1 object.

When commiting the merged transaction, the following exception is thrown:
Caused by: java.lang.IllegalStateException: This commit deletes object OID2 and adds a reference at the same time
	at org.eclipse.emf.cdo.internal.server.TransactionCommitContext.lockTarget(TransactionCommitContext.java:834)
	at org.eclipse.emf.cdo.internal.server.TransactionCommitContext.access$8(TransactionCommitContext.java:816)
	at org.eclipse.emf.cdo.internal.server.TransactionCommitContext$5.visit(TransactionCommitContext.java:660)
	at org.eclipse.emf.cdo.internal.common.revision.delta.CDOAddFeatureDeltaImpl.accept(CDOAddFeatureDeltaImpl.java:55)
	at org.eclipse.emf.cdo.spi.common.revision.CDOFeatureDeltaVisitorImpl.visit(CDOFeatureDeltaVisitorImpl.java:53)
...

The result-changeSet after the merge is correct containing 1 detached object (level2), and 2 changed objects (level1 & level3).
The problem seems to be in the DetachTransition which is called in CDOTransaction.applyChangeSetData adding all containments to the detached list of the transaction, resulting in the IllegalStateException.


Reproducible: Always

Steps to Reproduce:
1. Create a 3 level deep containment tree, create a branch.
2. Delete level2 and reattach level3 to level1 on branch.
3. Merge, commit.
Comment 1 Pascal Lehmann CLA 2010-09-21 11:10:34 EDT 
Created attachment 179321 [details]
Testcase
Comment 2 Pascal Lehmann CLA 2010-09-21 11:13:33 EDT 
1) The number of lines that you 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 3 Pascal Lehmann CLA 2010-09-22 10:33:59 EDT 
Created attachment 179382 [details]
Proposed patch

As changeSets do not hold the reattched objects seperately, I tried rebuilding this information using the dirty and detached lists.

1) The number of lines that you 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 Eike Stepper CLA 2010-09-23 03:56:42 EDT 
Created attachment 179438 [details]
Combined patch - for future reference

Good catch, Pascal!
Comment 5 Eike Stepper CLA 2010-09-23 04:12:11 EDT 
Committed to HEAD
Comment 6 Pascal Lehmann CLA 2010-12-16 03:11:20 EST 
Reopened.
I noticed the provided patch does not solve the problem in all cases. E.g. when the re-attached object itself contains other objects which are either modified or re-attached somewhere else, one of the objects get's re-attached twice and the following transition error occurs:

java.lang.IllegalStateException: Failing event PREPARE in state DIRTY for GenRefMultiContained@OID7 (data=Pair[CDOTransaction[2:1], []])
	at org.eclipse.net4j.util.fsm.FiniteStateMachine.process(FiniteStateMachine.java:153)
	at org.eclipse.emf.internal.cdo.CDOStateMachine.prepare(CDOStateMachine.java:229)
	at org.eclipse.emf.internal.cdo.CDOStateMachine.attach(CDOStateMachine.java:191)
	at org.eclipse.emf.internal.cdo.transaction.CDOTransactionImpl.applyChangeSetData(CDOTransactionImpl.java:587)
	at org.eclipse.emf.internal.cdo.transaction.CDOTransactionImpl.merge(CDOTransactionImpl.java:437)
Comment 7 Pascal Lehmann CLA 2010-12-16 03:13:21 EST 
Created attachment 185297 [details]
Testcase v2

Updated testcase which shows the problem.
Comment 8 Eike Stepper CLA 2012-06-05 07:30:20 EDT 
Moving all open bug reports to 4.1 because the release is very near and it's hghly unlikely that there will be spare time to address 4.0 problems.

Please make sure that your patches can be applied against the master branch and that your problem is not already fixed there!!!
Comment 9 Eike Stepper CLA 2012-08-14 22:53:32 EDT 
Moving all open issues to 4.2. Open bugs can be ported to 4.1 maintenance after they've been fixed in master.
Comment 10 Eike Stepper CLA 2012-10-31 09:28:16 EDT 
CDOTransactionImpl.applyChangedObjects() had this code:

        // handle reattached objects.
        if (detachedObjects.containsKey(id))
        {
          CDOStateMachine.INSTANCE.attach(object, this);
        }

That caused unpredictable problems (state could be either DIRTY or CLEAN, but never TRANSIENT). The reason is that both the attach and the reattach transitions are recursive, i.e., self-apply to contained objects. If the contained objects are in the to-be-applied merge result themselves individually then they're handled/attached multiple times, having a state at that time that depends on the order in the merge result.
Comment 11 Eike Stepper CLA 2012-10-31 09:30:59 EDT 
The solution is to factor out the relevant code from ReattachTransition (all but the recursion) into a separate method: CDOStateMachine.internalReattach(). This method must be called directly from CDOTransactionImpl.applyChangeSet().
Comment 12 Eike Stepper CLA 2012-10-31 09:39:11 EDT 
All tests are passing.

commit ccd00426290e441a0bbfc367ee5558acdd76338d
Comment 13 Eike Stepper CLA 2013-06-27 03:33:11 EDT 
Available in R20130613-1157 (4.2)