Bug 461802 - IndexOutOfBoundsException with nested left join fetches defined through orm.xml hints
Summary: IndexOutOfBoundsException with nested left join fetches defined through orm.x...
Status: NEW
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Eclipselink (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 normal with 3 votes (vote)
Target Milestone: ---   Edit
Assignee: Nobody - feel free to take it CLA
QA Contact:
URL:
Whiteboard:
Keywords: vm
Depends on:
Blocks:
 
Reported: 2015-03-10 07:47 EDT by Gonzalo Peche CLA
Modified: 2022-06-09 10:20 EDT (History)
4 users (show)

See Also:


Attachments
Maven project with test cases and patched classes (45.57 KB, application/zip)
2015-03-10 07:47 EDT, Gonzalo Peche CLA
no flags Details
Maven projects and test cases to support 2.4.2 version (80.90 KB, application/octet-stream)
2021-02-27 05:25 EST, Miklos Krivan CLA
no flags Details
Maven project with 2.7.4 support for testing (93.35 KB, application/x-zip-compressed)
2022-01-04 14:44 EST, Miklos Krivan CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gonzalo Peche CLA 2015-03-10 07:47:37 EDT
Created attachment 251422 [details]
Maven project with test cases and patched classes

Specifying nested left join fetches by putting "eclipselink.join-fetch" and "eclipselink.left-join-fetch" in orm.xml such as:

 <query>from T1 t1</query>
 <hint name="eclipselink.join-fetch" value="t1.fk2"/>
 <hint name="eclipselink.left-join-fetch" value="t1.fk2.fk3"/>

can fail with a IndexOutOfBoundsException. The typical stack trace would be something like:

java.lang.IndexOutOfBoundsException: Index: 2, Size: 1
	at java.util.ArrayList.rangeCheckForAdd(ArrayList.java:661)
	at java.util.ArrayList.add(ArrayList.java:473)
	at org.eclipse.persistence.internal.queries.JoinedAttributeManager.addExpressionAndBaseToGroupedList(JoinedAttributeManager.java:843)
	at org.eclipse.persistence.internal.queries.JoinedAttributeManager.prepareJoinExpressions(JoinedAttributeManager.java:794)
	at org.eclipse.persistence.queries.ObjectLevelReadQuery.prePrepare(ObjectLevelReadQuery.java:2085)
	at org.eclipse.persistence.queries.ObjectLevelReadQuery.checkPrePrepare(ObjectLevelReadQuery.java:918)
	at org.eclipse.persistence.queries.ObjectLevelReadQuery.checkPrepare(ObjectLevelReadQuery.java:899)
	at org.eclipse.persistence.queries.DatabaseQuery.checkPrepare(DatabaseQuery.java:613)
	at org.eclipse.persistence.internal.jpa.QueryImpl.getDatabaseQueryInternal(QueryImpl.java:341)
	at org.eclipse.persistence.internal.jpa.EntityManagerImpl.createNamedQuery(EntityManagerImpl.java:1124)


The problem is in org.eclipse.persistence.internal.jpa.metadata.queries.NamedQueryMetadata.processHints(...):

    protected Map<String, Object> processQueryHints(AbstractSession session) {
        Map<String, Object> hints = new HashMap<String, Object>();
        ...

That HashMap is iterated later when hints are applied, and code assumes that iteration happens in insertion order. Just changing the HashMap to a LinkedHashMap fixes the problem.

The bug is somewhat masked because in JDK6/JDK7, HashMap happens to put "eclipselink.join-fetch" hints before "eclipselink.left-join-fetch" ones unless the HashMap grows unrealistically big. But in JDK8 (or JDK7 with -Djdk.map.althashing.threshold=0, see http://docs.oracle.com/javase/7/docs/technotes/guides/collections/changes7.html) the hashing changes, and the bug is much easier to trigger.

A workaround is specifying those hints through Query.setHint(...), as this bypasses said Map completely, but that forces the developer either to maintain queries and hints separately, or to just avoid orm.xml and create the queries in code.

Attached is a Maven project with some tests that help to reproduce the problem. Also, patched versions of the affected classes are included:

- org.eclipse.persistence.internal.jpa.metadata.queries.NamedQueryMetadata: this is where the real problem is (it should use LinkedHashMap instead of HashMap to "sort of" maintain the order of hints in orm.xml).

- org.eclipse.persistence.internal.queries.JoinedAttributeManager: the addExpressionAndBaseToGroupedList can be made more robust so that no IndexOutOfBoundException is thrown. Anyway this is not necessary if NamedQueryMetadata is patched.

Sources for these two classes have the ".java.patched" extension. By removing the ".patched" part and relaunching the tests, the corrections can be tested without rebuilding Eclipselink. Either one of the included fixed classes make the tests pass, but I think that the real fix is the one in NamedQueryMetadata.

TESTED JDKs: 
- Oracle JDK 6
- Oracle JDK 7, Open JDK 7
- Oracle JDK 7 + jdk.map.althashing.threshold=0, Open JDK 7 + jdk.map.althashing.threshold=0 
- Oracle JDK 8 (Open JDK 8)

EXPECTED RESULTS: All tests succeed in all JDKs

ACTUAL RESULTS:

shouldNotThrowIndexOutOfBoundsExceptionOnJoinedAttributeManager_CommonCase:
   fails in JDK8 (always) and JDK 7 + jdk.map.althashing.threshold=0 (randomly)

shouldNotThrowIndexOutOfBoundsExceptionOnJoinedAttributeManager_UnrealisticHintSet:
   fails in JDK6 (always), JDK7 (always) and JDK 7 + jdk.map.althashing.threshold=0 (randomly)

shouldNotThrowIndexOutOfBoundsExceptionOnJoinedAttributeManager_Workaround:
   succeeds in all JDKs
Comment 1 Klemen Ferjancic CLA 2018-07-23 07:42:31 EDT
I got exact same error but using setHint in code. The problem is that I used Map<Object, String> as initial storage for hints and iterated over it, calling setHint on each entry. My guess is that EclipseLink stores references to hints therefore I got exact same error as described here. Perhaps it would be safer if it made internal copies.
Comment 2 Miklos Krivan CLA 2021-02-27 05:25:58 EST
Created attachment 285685 [details]
Maven projects and test cases to support 2.4.2 version

Small changes added to Gonzalo Peche's maven projects and test cases to support 2.4.2 version and also I have checked the 2.6.9 as well. The problem appears in all theese three versions and not only with orm.xml but when QueryHints added to named queries as well.
Comment 3 Miklos Krivan CLA 2021-02-27 05:37:59 EST
Many thanks to Gonzalo Peche for the patch given in a maven test project. 

I have downloaded and tested with diffeent eclipselink versions. The problem solved by this patch which still exists in the 2.6.9 version as well when running in a Java 8 environment.

I have attached my modified maven project. I had to change just a small thing in the original JPQL query to support the older 2.4.2 as well.

In my case it was not enough just patching the NamedQueryMetadata but also the JoinedAttributeManager as well. I had no enough time to analyze this case. 

It was a big help to me because I have to migrate our glassfish 3.1 application from Java 7 to Java 8. First I have upgraded the eclipselink version using the official OSGI package from 2.3 to 2.4 then I have realized the situation so I had to repair it.

I am going to visit the latest 2.7.x and 3.0.x versions as well.
Comment 4 Miklos Krivan CLA 2022-01-04 14:44:07 EST
Created attachment 287777 [details]
Maven project with 2.7.4 support for testing

I had to do one more step because I want to use the Eclipse supported GlassFish 5.1.0 application server with JavaEE 8 features. And I have faced with the same problem again after a year. 

It uses the eclipselink 2.7.4 version so I had to add to the example project some small modification to support 2.7.4 wich has signed jars so I had to include the javax.persistence artifact as well before the eclipselink artifact itself in the pom (this was the only workaround).

With this test I could verify that Gonzalo Peche patch works for this version as well. And based on the result I have modified the two osgi jar file with the newly compiled two patched java classes (core and jpa library) in the Glassfish modules folder. 

And now my application works in GlassFish 5.1.0 as well.

This patch is required because I am using @QueryHint annotation in many @NamedQuery definitions. And the problem still exists with JDK 8. Right now I do not want to change this approach in all places in my project into the explicit query.setHint() method calls wich works without any patch as Gonzalo mentioned as well.

I hope that this problem is also important for many others and probably soon it will be appear a final accepted and approved solution in a published future version as well.
Comment 5 Radek Felcman CLA 2022-02-14 09:29:55 EST
Additional bug and bugfix for master, 3.0, 2.7 branches
https://github.com/eclipse-ee4j/eclipselink/issues/1148
Comment 6 Eclipse Webmaster CLA 2022-06-09 10:15:30 EDT
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink
Comment 7 Eclipse Webmaster CLA 2022-06-09 10:20:40 EDT
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink