Bug 316444

Comment 1 Martin Fluegge 2010-06-13 11:31:32 EDT

I extended the test model 3 to provide simple model classes which allow building up a containment tree. The model now additionally contains two classes:

NodeA ? contains a containment feature called ?children?. The up-reference to the parent is implemented using the eContainer.

NodeB - contains a containment feature called ?children? and a feature called ?parent? which are both connected with a bidirectional containment reference. 

The model enhancement is also regenerated for legacy.

Committed to HEAD.

Comment 2 Martin Fluegge 2010-06-13 13:29:29 EDT

Created attachment 171805 [details]
Test v1

I started creating a test to reproduce the problem. Currently it contains simple non-concurrency tests, which seems to work fine. 

Now I am going to find the problem in a multi-threaded surrounding.

Comment 3 Martin Fluegge 2010-06-20 07:15:16 EDT

Created attachment 172290 [details]
Test v2

I updated the test (and test environment) to check the concurrent access to the repository. Now we can reproduce the inconsistency of the repository. After both threads were executing their changes simultaneously the graph is destroyed, means that objects are lost.

Comment 4 Martin Fluegge 2010-06-28 07:59:17 EDT

Created attachment 172891 [details]
Patch v1

I created a hot fix for the problem which avoids the containment cycles. The performance of this solution is rather poor. I am going to optimize it in the next step. 

All also changed the test case that it now passes with the hot fix and fails without instead of just validating the consistency of the graph.

Comment 5 Martin Fluegge 2010-06-28 08:00:07 EDT

Created attachment 172892 [details]
Test v3

Here comes the test patch

Comment 6 Eike Stepper 2010-06-29 04:51:16 EDT

Rebasing all outstanding enhancements requests to version 4.0

Comment 7 Martin Fluegge 2010-07-01 05:22:52 EDT

Created attachment 173188 [details]
Patch v2

I modified the last approach a bit. Now the performance should be much better because now an exception is thrown on the first locked parent in the graph.

Comment 8 Martin Fluegge 2010-07-01 12:21:18 EDT

Created attachment 173234 [details]
Patch v3

The error detection behavior has been optimized. In the last patch an exception also occurred if the parent object on the graph was lock due to an EAttribute change. This is now detected. This means that the transaction is only rejected if the was a movement in the graph. 
With this patch the bug is solved from my point of view.

Comment 9 Martin Fluegge 2010-07-01 12:22:18 EDT

Created attachment 173235 [details]
Test v3

This test validates the patch v3 including the EAttribute problem.

Comment 10 Martin Fluegge 2010-07-01 12:24:03 EDT

Created attachment 173236 [details]
Test v4

Sorry. It should be Test v4

Comment 11 Martin Fluegge 2010-07-02 04:56:19 EDT

Created attachment 173278 [details]
Patch v4

I modified the API changes a bit so net4j is now untouched again.The getObjects() will now be called from the LockManager. To avoid the cast to LockManager the interface InternalLockManager now defines the method.

Comment 12 Martin Fluegge 2010-07-02 04:56:54 EDT

Created attachment 173279 [details]
Test v5

Comment 13 Eike Stepper 2010-07-02 07:38:27 EDT

Created attachment 173291 [details]
Test+Fix - ready to be committed

Well done, Martin ;-)

Comment 14 Eike Stepper 2010-07-02 08:24:36 EDT

Created attachment 173297 [details]
Fix+Test - ready to be committed

I have made this new validation optional with the IRepository.Props.ENSURE_REFERENTIAL_INTEGRITY server configuration option

Comment 15 Martin Fluegge 2010-07-02 10:29:22 EDT

Committed to HEAD.

Note that the fix only works with branching configuration. Non branching configurations might fail on the containment cycle detection. 
To enable the repository to check for containment cycles you must activate the following property

props.put(IRepository.Props.ENSURE_REFERENTIAL_INTEGRITY, "true");

Comment 16 Martin Fluegge 2010-07-02 10:38:34 EDT

The fixing of the non branching configs will be handled in Bug 318729

Comment 17 Eike Stepper 2010-07-05 05:44:18 EDT

Correction:

This fix is NOT configurable at runtime!!!

I mixed that up with bug 316434 ;-(

Comment 18 Eike Stepper 2011-06-23 03:42:16 EDT

Available in R20110608-1407

Comment 19 Eike Stepper 2013-05-29 04:29:33 EDT

See also bug 409284 (4.2).