Document History

Date

Author

Version Description & Notes

20080105

Michael O'Brien

1.0 Initial reproduction use cases

20080111

Michael O'Brien

1.1 Simplify reproduction for flush on query

Overview

This bug describes the behavior and fix for the issue of nested flush() calls when using the @PrePersist annotation callback method on an unmanaged entity containing a read query method that causes a flush() when the entity is the target of a relationship from a managed entity that has pending changes.

This bug is also encountered when a secondary persist is executed inside of a @PrePersist callback method on the Entity being itself persisted. Soo there are two use cases here - we will concentrate on the read query causing a secondary flush to sync changes before the read - causing an infinite loop or double persist which causes a PK conflict.

Concepts

See p.139 of Pro EJB 3.0"Some providers will flush the persistence context to ensure that the query incorporates all pending changes" - EclipseLink is one of these providers.

JPA Specification Notes

Here is what the JPA specification says about performing persists inside the @PrePersist method. Essentially it states that we should not be performing persists inside a @PrePersist callback - however it may be the responsibility of the container to handle any that may be done depending on the nature of the persistence.

JPA 1.0 Specification

P58 Section 3.5

"In general, portable applications should not invoke EntityManager or Query operations, access other entity instances, or modify relationships in a lifecycle callback method.[19]"

"[19] The semantics of such operations may be standardized in a future release of this specification."

JPA 2.0 Specification

P82 Section 3.5.1

"In general, portable applications should not invoke EntityManager or Query operations, access other entity instances, or modify relationships within the same persistence context.[34] A lifecycle callback method may modify the non-relationship state of the entity on which it is invoked."

"[34] The semantics of such operations may be standardized in a future release of this specification."

Reproduction

Prerequisites

Run tests out of container on an SE persistence unit (EE can also be used)

At least two entities must be persisted to avoid performance optimization code for a single entity

Two different entity classes are required

One must be managed, the other one containing the @PrePersist must be unmanaged/detached but referenced by the first

Test must commit more than one object so that the non-performance else clause in CommitManager.commitAllObjectsWithChangeSet() is used

Test must perform a change on an existing object that is already persisted

Logs

[EL Finest]: 2009.01.05 13:09:22.915--UnitOfWork(2804823)--Thread(Thread[main,5,main])--assign sequence to the object (1,210 -> org.eclipse.persistence.example.business.Cell@11024915( id: null state: null left: null right: null))
[EL Finest]: 2009.01.05 13:09:22.915--UnitOfWork(2804823)--Thread(Thread[main,5,main])--PERSIST operation called on: org.eclipse.persistence.example.business.Cell@8180602( id: null state: null left: null right: null).
[EL Finest]: 2009.01.05 13:09:22.930--UnitOfWork(2804823)--Thread(Thread[main,5,main])--assign sequence to the object (1,211 -> org.eclipse.persistence.example.business.Cell@8180602( id: null state: null left: null right: null))
Exception in thread "main" java.lang.StackOverflowError
at java.text.DecimalFormat.setMinimumIntegerDigits(DecimalFormat.java:2677)
at java.text.SimpleDateFormat.zeroPaddingNumber(SimpleDateFormat.java:1184)
at java.text.SimpleDateFormat.subFormat(SimpleDateFormat.java:1125)
at java.text.SimpleDateFormat.format(SimpleDateFormat.java:882)
at java.text.SimpleDateFormat.format(SimpleDateFormat.java:852)
at java.text.DateFormat.format(DateFormat.java:316)
at org.eclipse.persistence.logging.AbstractSessionLog.getDateString(AbstractSessionLog.java:644)
at org.eclipse.persistence.logging.AbstractSessionLog.getSupplementDetailString(AbstractSessionLog.java:654)
at org.eclipse.persistence.logging.DefaultSessionLog.log(DefaultSessionLog.java:130)
at org.eclipse.persistence.internal.sessions.AbstractSession.log(AbstractSession.java:2492)
at org.eclipse.persistence.internal.sessions.AbstractSession.log(AbstractSession.java:3579)
at org.eclipse.persistence.internal.sessions.AbstractSession.log(AbstractSession.java:3551)
at org.eclipse.persistence.internal.sessions.AbstractSession.log(AbstractSession.java:3527)
at org.eclipse.persistence.internal.sessions.AbstractSession.log(AbstractSession.java:3449)
at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.logDebugMessage(UnitOfWorkImpl.java:5106)
at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.registerNotRegisteredNewObjectForPersist(UnitOfWorkImpl.java:3945)
at org.eclipse.persistence.internal.sessions.RepeatableWriteUnitOfWork.registerNotRegisteredNewObjectForPersist(RepeatableWriteUnitOfWork.java:336)
at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.registerNewObjectForPersist(UnitOfWorkImpl.java:3903)
at org.eclipse.persistence.internal.jpa.EntityManagerImpl.persist(EntityManagerImpl.java:254)
at org.eclipse.persistence.example.business.Cell.processInsert(Cell.java:121)
at org.eclipse.persistence.example.business.Cell.prePersist(Cell.java:69)
at sun.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.eclipse.persistence.internal.security.PrivilegedAccessHelper.invokeMethod(PrivilegedAccessHelper.java:344)
at org.eclipse.persistence.internal.jpa.metadata.listeners.EntityListener.invokeMethod(EntityListener.java:297)
at org.eclipse.persistence.internal.jpa.metadata.listeners.EntityClassListener.invokeMethod(EntityClassListener.java:64)
at org.eclipse.persistence.internal.jpa.metadata.listeners.EntityListener.prePersist(EntityListener.java:412)
at org.eclipse.persistence.descriptors.DescriptorEventManager.notifyListener(DescriptorEventManager.java:650)
at org.eclipse.persistence.descriptors.DescriptorEventManager.notifyEJB30Listeners(DescriptorEventManager.java:593)
at org.eclipse.persistence.descriptors.DescriptorEventManager.executeEvent(DescriptorEventManager.java:187)
at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.registerNewObjectClone(UnitOfWorkImpl.java:3980)
at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.registerNotRegisteredNewObjectForPersist(UnitOfWorkImpl.java:3959)
at org.eclipse.persistence.internal.sessions.RepeatableWriteUnitOfWork.registerNotRegisteredNewObjectForPersist(RepeatableWriteUnitOfWork.java:336)
at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.registerNewObjectForPersist(UnitOfWorkImpl.java:3903)
at org.eclipse.persistence.internal.jpa.EntityManagerImpl.persist(EntityManagerImpl.java:254)
at org.eclipse.persistence.example.business.Cell.processInsert(Cell.java:121)
at org.eclipse.persistence.example.business.Cell.prePersist(Cell.java:69)
at sun.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.eclipse.persistence.internal.security.PrivilegedAccessHelper.invokeMethod(PrivilegedAccessHelper.java:344)
at org.eclipse.persistence.internal.jpa.metadata.listeners.EntityListener.invokeMethod(EntityListener.java:297)
at org.eclipse.persistence.internal.jpa.metadata.listeners.EntityClassListener.invokeMethod(EntityClassListener.java:64)
at org.eclipse.persistence.internal.jpa.metadata.listeners.EntityListener.prePersist(EntityListener.java:412)
at org.eclipse.persistence.descriptors.DescriptorEventManager.notifyListener(DescriptorEventManager.java:650)
at org.eclipse.persistence.descriptors.DescriptorEventManager.notifyEJB30Listeners(DescriptorEventManager.java:593)
at org.eclipse.persistence.descriptors.DescriptorEventManager.executeEvent(DescriptorEventManager.java:187)
at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.registerNewObjectClone(UnitOfWorkImpl.java:3980)
at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.registerNotRegisteredNewObjectForPersist(UnitOfWorkImpl.java:3959)
at org.eclipse.persistence.internal.sessions.RepeatableWriteUnitOfWork.registerNotRegisteredNewObjectForPersist(RepeatableWriteUnitOfWork.java:336)
at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.registerNewObjectForPersist(UnitOfWorkImpl.java:3903)
at org.eclipse.persistence.internal.jpa.EntityManagerImpl.persist(EntityManagerImpl.java:254)
at org.eclipse.persistence.example.business.Cell.processInsert(Cell.java:121)
at org.eclipse.persistence.example.business.Cell.prePersist(Cell.java:69)
at sun.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)

Use Case 3: Persist other Entity (with a @PrePersist without a persist) inside @PrePersist callback

The @PrePersist of the first entity will cause a persist of the second entity that also has it's own @PrePersist (but without another persist).

Normal functionality with no infinite loop.

Use Case 4: Persist other Entity (without its own @PrePersist) inside @PrePersist

Normal functionality with no infinite loop.

Use Case 5: @PrePersist calls another @PrePersist on a referenced entity

Analysis Constraints

Concurrency and Thread Safety

The entityManagerFactory is thread-safe, the entityManager is not. However, the boolean flag withinFlush is a non-static field on each instance of EntityManager so as long as the lifecyle of the entityManager is thread-safe then the addition of this flag will have no issues.

Design / Functionality

Alternative 1: No flush() within a flush()

We will not perform nested flush() calls, a break will be required and a warning will printed if the entityManager is in FlushModeType.AUTO.

We will accomplish this by adding a boolean flag withinFlush to the EntityManagerImpl class that will be true for the lifetime of any flush() call. We will check this flag before issuing a flush in the preparation within EJBQueryImpl.executeReadQuery() - which occurs in the case that a query requires pending uncommitted changes written before running it's query. This would happen if a query or persist were done inside of a @PrePersist callback method.

- deprecated in favor of alternative 2:

Alternative 2: No nested flush in RepeatableWriteUnitOfWork

The call to calculateChanges() can be recursive before we even get to commitToDatabaseWithPreBuiltChangeSet().
In this case we will commit in a PrePersist and then later fail on a second commit.
We therefore need to move the flag set before the try/catch block.

we now set the flag before calculateChanges and clear it in two places

- when we dont commit anything on an early return - just before the return

Implementation

A new boolean field will be introduced on EntityManagerImpl.java

/** Track whether we are already in a flush() */protectedboolean withinFlush;

that will be set inside flush() just before writeChanges() to true and then reset to false on the finaly block. The use of this flag will be thread safe as far as the entityManager instance is used in a thread safe manner.

EJBQueryImpl when executing a read (possibly from a @PrePersist callback) will check that it is not already in a flush() before doing a pre-flush() - normally done so that any outstanding (uncommitted) changes are included in this query.

[EL Warning]: 2009.01.11 21:36:21.786--Thread(Thread[main,5,main])--The em: org.eclipse.persistence.internal.jpa.EntityManagerImpl@1777b1 is already flushing. The query will be executed without further changes being written to the database. If the query is conditional upon changed data the changes may not be reflected in the results. Users should issue a flush() call upon completion of the dependent changes and prior to this flush() to ensure correct results.

Testing

JPA LRG passes

Core LRG passes

Static Weaved reproduction passes

Use Case 1: Output of nested flush()

The following logs and stacktrace illustrate the current nested flush() behavior when performing a @PrePersist containing a query that requires a flush() on uncommitted managed entity changes.

Run static weaver

Add the javaagent instrumentation flag to run target

Increase the Native Stack Size using the -Xss JVM parameter

You will decrease the chance of getting a StackOverflowError for highly connected Entity models like rings or bidirectional trees or arrays by increasing the native stack size.

I have found that increasing the stack to the value below pushed the StackOverflowError experienced in a bidirectionally linked 6 dimensional Hypercube model (64 entities) to a 13 dimensional Hypercube (8192 entities) in bug# 310662.

-Xss42m (Native cache size)

-Xoss42m had no effect (Java cache size)

Documentation

Open Issues

Issue #

Owner

Description / Notes

I1

mobrien

Verify that Entity containing the @PrePersist callback has been registered

- yes the target entity is registered and committed but changes to the source may not appear in the query results in the @PrePersist

Decisions

Issue #

Description / Notes

Decision

Future Considerations

During the research for this bug, the following items were identified as out of scope but are captured here as potential future enhancements. If agreed upon during the review process these should be logged in the bug system.