NPE in QueryBasedValueHolder due to #2181

[Julien-Hanin]

Description

When populating the second-level cache, eclipselink 4.0.4 sometimes throws

Exception in thread "Thread-2" java.lang.NullPointerException: Cannot invoke "org.eclipse.persistence.queries.ReadQuery.isReadObjectQuery()" because "this.query" is null
at org.eclipse.persistence.internal.indirection.QueryBasedValueHolder.getValue(QueryBasedValueHolder.java:102)
at org.eclipse.persistence.internal.indirection.UnitOfWorkValueHolder.instantiateImpl(UnitOfWorkValueHolder.java:176)
at org.eclipse.persistence.internal.indirection.UnitOfWorkValueHolder.instantiate(UnitOfWorkValueHolder.java:252)
at org.eclipse.persistence.internal.indirection.DatabaseValueHolder.getValue(DatabaseValueHolder.java:109)
at concurrency.Employee._persistence_get_department(Employee.java)
at concurrency.Employee.getDepartment(Employee.java:42)
at concurrency.Main.lambda$main$0(Main.java:23)
at java.base/java.lang.Thread.run(Thread.java:840)

There is no exception with version 4.0.3. The change that added the bug is #2181.

To Reproduce

1. Clone the repo https://github.com/Julien-Hanin/eclipselink-query-is-null
2. Add a breakpoint (suspending only the current thread) on line 102 of org.eclipse.persistence.internal.indirection::QueryBasedValueHolder.
3. Start debugging concurrency.Main with the java agent enabling weaving
4. Resume program twice when breakpoint is reached

This results in the exception.

Explanation

The problem is UnitOfWorkValueHolder uses its "wrappedValueHolderLock" to lock its wrapped queryBasedValueHolder instead of using the queryBasedValueHolder's lock.

I've provided a fix (an ugly one) to show this is the issue. You can use it by removing the "_2" at the end of the provided files DatabaseValueHolder_2 and UnitOfWorkValueHolder_2.

Potential additional issue

The value holders are clonable and their clone method only makes a shallow-copy of the fields. So, after a clone, two cloned value holders have the same lock.
On the other hand, "synchronized(this)" in 4.0.3 synchronizes on two different objects (the two cloned objects). I didn't investigate further though.

[vreuland]

Hello,

Experiencing the same issue, It looks like it is indeed introduced by #2181 and the move from synchronized to ReentrantLocks.
The analysis/explaination of @Julien-Hanin make senses when we look at the changes made in https://github.com/eclipse-ee4j/eclipselink/pull/2181/files#diff-143d2734122d2b31420d56dc98d236847b0e786f446385bce909432977d946f4L149

The problem is UnitOfWorkValueHolder uses its "wrappedValueHolderLock" to lock its wrapped queryBasedValueHolder instead of using the queryBasedValueHolder's lock.

I see other issues also pointing #2181 as potential root cause (like #2219).
Is simply reverting #2181 and releasing a 4.0.5 an option? Or can this be easily and quickly fixed keeping the RentrantLocks? I see @Julien-Hanin is proposing a fix.

Thank you in advance.
Kind regards,
Vincent

[rfelcman]

First release candidate of EclipseLink 4.0.5 (4.0.5-RC1) is available.
You can download it from usual sources like:
Maven Central
Milestone builds: https://download.eclipse.org/rt/eclipselink/milestones/4.0.5/RC1/
List of changes against 4.0.4 is available at: https://github.com/eclipse-ee4j/eclipselink/releases/tag/4.0.5-RC1

[vreuland]

Great 👍
Thank you very much @rfelcman

[rfelcman]

Fixed in 4.0 branch, please reopen issue or leave message there is issue remains.