Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 321041 - EclipseLink throws ConcurrentModificationException when triggering lazy load from conforming query
Summary: EclipseLink throws ConcurrentModificationException when triggering lazy load ...
Status: CLOSED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Eclipselink (show other bugs)
Version: unspecified   Edit
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---   Edit
Assignee: Nobody - feel free to take it CLA
QA Contact:
URL:
Whiteboard: submitted_patch
Keywords:
Depends on:
Blocks: 522635
  Show dependency tree
 
Reported: 2010-07-27 11:55 EDT by Mark Wolochuk CLA
Modified: 2022-06-09 10:25 EDT (History)
5 users (show)

See Also:


Attachments
Proposed patch (2.35 KB, patch)
2010-08-10 17:03 EDT, Mark Wolochuk CLA
peter.krogh: iplog+
Details | Diff
Stack trace (1.62 KB, text/plain)
2010-08-12 15:31 EDT, Mark Wolochuk CLA
no flags Details
Code that reproduces the bug (2.74 KB, application/zip)
2010-08-12 17:09 EDT, Mark Wolochuk CLA
no flags Details
Eclipse project to recreate bug (6.87 MB, application/zip)
2010-08-16 16:55 EDT, Mark Wolochuk CLA
no flags Details
Updated patch (8.26 KB, patch)
2010-08-17 14:58 EDT, Tom Ware CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wolochuk CLA 2010-07-27 11:55:18 EDT
When doing a conforming query on an attribute that is lazy-loaded via value holder, and the attribute is of the same type as the parent, EclipseLink will throw a ConcurrentModificationException. The exception occurs when EclipseLink iterates over the unit of work identity map, then triggers a lazy load of a child attribute, adds it to the map, then attempts to continue iterating.

The code below reproduces this bug:

// Parent class contains an instance of child same type
public class JoeType {
   private JoeType joeChild; 
}

public JoeType findParentByChildId(Integer childId) {
    ExpressionBuilder fieldExpression = new ExpressionBuilder();
    Expression exp = fieldExpression.get("joeChild").get("id").equal(childId);
    ReadObjectQuery q = new ReadObjectQuery(JoeType.class, exp);
    q.conformResultsInUnitOfWork();
    q.setInMemoryQueryIndirectionPolicyState(InMemoryQueryIndirectionPolicy.SHOULD_TRIGGER_INDIRECTION);
    return (JoeType) getUnitOfWork().executeQuery(q);
}

@Test
public void testConcurrentModificationExceptionBug()
{
    // Load two parents by ID and register in unit of work
    Expression expression1 = new ExpressionBuilder().get("id").equal(PARENT1_ID);
    JoeType parent1 = session.readObject(JoeType.class, expression1);
    uow.registerObject(parent1);
    
    Expression expression2 = new ExpressionBuilder().get("id").equal(PARENT2_ID);
    JoeType parent2 = session.readObject(JoeType.class, expression2);
    uow.registerObject(parent2);
    
    // This will throw ConcurrentModificationException
    // when parent1 is iterated over first in the cache
    // and child1 lazy load is triggered and EclipseLink
    // adds child1 into the cache then tries to continue
    // iterating.
    Parent foundParent = findParentByChildId(CHILD2_ID); 
}
Comment 1 Tom Ware CLA 2010-08-09 14:00:24 EDT
Setting target and priority.  See the following page for details of the meanings of these fields:

http://wiki.eclipse.org/EclipseLink/Development/Bugs/Guidelines
Comment 2 Mark Wolochuk CLA 2010-08-10 17:03:22 EDT
Created attachment 176282 [details]
Proposed patch

My proposed solution is to create a copy of the cache key collection and iterate over that in IdentityMapManager if in memory policy is to trigger indirection.
Comment 3 Tom Ware CLA 2010-08-12 14:13:48 EDT
Triaging because of submitted patch - 'will assign to myself and either provide feedback and reassign, or check-in.
Comment 4 Tom Ware CLA 2010-08-12 14:59:01 EDT
Does this issue occur in more recent versions? (2.1 for instance)  My initial attempt at recreating this issue on the 2.1 stream was not successful.
Comment 5 Mark Wolochuk CLA 2010-08-12 15:29:08 EDT
(In reply to comment #4)
> Does this issue occur in more recent versions? (2.1 for instance)  My initial
> attempt at recreating this issue on the 2.1 stream was not successful.

Yes, I just confirmed that I can repeat the problem in the 2.1 stream.

In the test, you may need to change which child you are querying on - if

Parent foundParent = findParentByChildId(CHILD2_ID);

does not cause the error, try

Parent foundParent = findParentByChildId(CHILD1_ID);

The error only occurs when the first object in the cache does not satisfy the query. Since it's a HashMap, iteration order is undefined, that's whey we don't know which is first.

Oh, I also just noticed that I didn't mention we are using ValueHolderInterface for the one-to-one child attribute:

public class JoeType {
   private ValueHolderInterface joeChild; // joeChild is a JoeType object
}
Comment 6 Mark Wolochuk CLA 2010-08-12 15:31:25 EDT
Created attachment 176495 [details]
Stack trace

Stack trace attached.
Comment 7 Tom Ware CLA 2010-08-12 15:37:49 EDT
Is it possible to attach your recreation?
Comment 8 Mark Wolochuk CLA 2010-08-12 17:09:11 EDT
Created attachment 176507 [details]
Code that reproduces the bug

The attached zip contains code to reproduce the bug, including sql to create and populate the required table. The table is populated with four rows - two parents and two children. The mappings are defined in the project file.

The unit test preloads parent1 and parent2 into a unit of work then searches by child id. There are two tests cases - one searches by child1 id, the other by child2 id. One will pass and one will fail, depending on which parent is first in the cache.

Note that there is a missing base class (IntegrationTestBase) which is our test harness. Otherwise, the code should be pretty close for you to get it into your test harness and reproduce the bug.
Comment 9 Tom Ware CLA 2010-08-16 10:57:45 EDT
I still cannot recreate.

I implemented getSession() to return a singleton session based on the provided EclipseLink project.

I tried implemented getUnitOfWork() either to return a singleton UOW based on the result of getSession() or to return a new UOW based on the result of getSession().

In both cases, my EclipseLink 2.1 code allows both testConcurrentModificationException1 and testConcurrentModificationException2 to return an object.  I see no exceptions.

What else can I do to recreate the issue? Can you try the latest 2.2.0 nightly build? (http://www.eclipse.org/eclipselink/downloads/nightly.php)
Comment 10 Mark Wolochuk CLA 2010-08-16 16:55:51 EDT
Created attachment 176738 [details]
Eclipse project to recreate bug

Sorry it's been so hard for you to reproduce. I am attaching a self-contained Eclipse project that reproduces the bug. I tested it with the latest nightly build of eclipselink.jar (2.2.0v20100814-r8039), and it does reproduce the bug Hopefully this one will work for you. All you need to do to get it running is:

1. Import the project into Eclipse.

2. Run the SQL to create and populate the database table.

3. Replace the tokens ***** in sessions.xml with your connection information (URL, username, password).

4. If necessary, change database driver jar to your own, and update classpath.

5. Run Bug321041Test (contains two tests).

One test will pass and the other will fail, demonstrating the bug. (I hope.)
Comment 11 Tom Ware CLA 2010-08-17 14:46:57 EDT
I have investigated the issue and am adding the fix as provided.  The only concern I have is related to the performance hit of copying the identity map keys into a list and, as mentioned in the comment, that is a fairly small hit compared to the cost of triggering valueholders, particularly if any of those triggers go to the DB.

Reviewed by Tom Ware with input from Andrei Ilitchev

Tested with full JPA and Core LRG

Adding TriggerValueHoldersSelfReferencingOneToOneTest to our Core IdentityMapTestSuite.
Comment 12 Tom Ware CLA 2010-08-17 14:58:33 EDT
Created attachment 176831 [details]
Updated patch
Comment 13 Tom Ware CLA 2010-08-18 09:07:33 EDT
Fix checked into trunk.  See above comments.
Comment 14 Peter Krogh CLA 2010-12-08 14:32:31 EST
moving ipLog flag to patch
Comment 15 Eclipse Webmaster CLA 2022-06-09 10:14:50 EDT
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink
Comment 16 Eclipse Webmaster CLA 2022-06-09 10:25:32 EDT
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink