Issue Details (XML | Word | Printable)

Key: HHH-2763
Type: Bug Bug
Status: Open Open
Priority: Critical Critical
Assignee: Steve Ebersole
Reporter: S.Schnabl
Votes: 71
Watchers: 74
Operations

If you were logged in you would be able to see more operations.
Hibernate Core

(lazy) m:n relation + EventListener = AssertionFailure: collection [n-side] was not processed by flush()

Created: 02/Aug/07 04:35 AM   Updated: 20/May/09 11:06 AM
Component/s: core
Affects Version/s: 3.2.4.sp1, 3.2.5
Fix Version/s: None

Time Tracking:
Not Specified

File Attachments: 1. Text File cdel_analyse.txt (12 kB)
2. Zip Archive HHH-2763_standalone_testcases.zip (39 kB)
3. Zip Archive Testcase.zip (4.30 MB)

Image Attachments:

1. bidirectional_many-to-many.gif
(7 kB)

2. one-to-many_with_many_side_having_collections.gif
(9 kB)
Environment: Windows-XP, Jboss 4.2.1GA, Hibernate 3.2.4SP1, EJB3
Issue Links:
Duplicate
 
Relates

Bug Testcase Reminder (view):
REMINDER: Bug reports should generally be accompanied by a test case
Participants: Adrian Pillinger, Ally Bongo, Brooks Lyrette, C. Delaloye, Darren Bell, Dave Knipp, Denis VERGNES, Eike Hirsch, Gail Badner, Geoffrey De Smet, Herman Chung, Jakob Braeuchi, Martin Backhaus, Michael Newcomb, nikita tovstoles, S.Schnabl, Samppa Saarela, Sanne Grinovero, Steve Ebersole, Tim Stavenger and Venkat Shanmugam


 Description  « Hide

For more details see the attached testcase. I'm sorry, but in the short of time i only got a testcase for jboss-server 4.2. Please deploy the server.ear from /release-directory and then call the /src/client/TestCaseClient.java.

[Summarized]
It seems, that touching a lazy (Persistent-)Collection of at least a m:n relation inside a Hibernate event-listener always raises this error:
org.hibernate.AssertionFailure: collection [n-side] was not processed by flush()

[Explanation]
I have two entities A. and B. Both having a m:n relation between each other. Furthermore there is an PostUpdateListener, which iterates onUpdate of entitiy through all properties of updated entity.

[Testcase]
Both entities are linked with eachother (m:n). If i now do a simple update of a property of entity A --> MyPostUpdateListener will be called, which iterates through every property of the updated entity. In case of this property was a collection (= lazy PersistentCollection of m:n relation), hibernate initializes the collection for further work. I can now run through all objects of the collection, but after all work is done in listener, I get the following exception from postFlush:

Caused by: org.hibernate.AssertionFailure: collection [com.qualitype.testcase.server.ejb.entity.EntityB.entitiesOfA] was not processed by flush()
at org.hibernate.engine.CollectionEntry.postFlush(CollectionEntry.java:205)
at org.hibernate.event.def.AbstractFlushingEventListener.postFlush(AbstractFlushingEventListener.java:333)
at org.hibernate.event.def.DefaultFlushEventListener.onFlush(DefaultFlushEventListener.java:28)
at org.hibernate.impl.SessionImpl.flush(SessionImpl.java:1000)
at org.hibernate.impl.SessionImpl.managedFlush(SessionImpl.java:338)
at org.hibernate.ejb.AbstractEntityManagerImpl$1.beforeCompletion(AbstractEntityManagerImpl.java:515)
... 29 more

Attention: EntityB.entitiesOfA is the other-side collection of the m:n relation of the updated EntityA.

We are using hibernate-event listener system for auditing-purposes, so you should understand that touching every (element in the) collection is necessary for audit-purposes.

Seems for me like a serious bug. Need this fixed asap ...



Geoffrey De Smet added a comment - 16/Aug/07 05:06 AM

I have this bug even without using a m:n relation:
My class A which validates using a property of its ManyToOne lazy relation to class B (which happen to have a OneToMany relation whichs breaks):

class A {
private B b; // ManyToOne

@AssertTrue
public boolean checkProps{ return prop1 == getB().getProp2(); }
}

The above probably works, unless B has a OneToMany lazy collection prop3.

I merge in a detached A a with a non-lazy detached b (which has a lazy prop3),
a becomes persistent, b becomes lazy.
During validation b becomes initialized (with prop3 lazy), but apparently too late as b's prop3 throws the above AssertionFailure.

The only workaround I see is using validation on the service layer instead of the hibernate layer, but our current architecture doesn't permit that.


Venkat Shanmugam added a comment - 11/Sep/07 10:12 AM

I have the same issue as I am trying to use listners to write history as our DB2 admins are not willing to create triggers. Will this be fixed soon or Is there any work around for this issue?


Venkat Shanmugam added a comment - 13/Sep/07 08:30 AM

I dig into the issue and realized flushing attribute may be the problem. So, I set flushing to true and false before and after calling executions respectively.

public void onPostInsert(PostInsertEvent event) {
try{
event.getSession().getPersistenceContext().setFlushing(true);
Iterator iter = listeners.iterator();
while( iter.hasNext() ){ ICUDListener icud = (ICUDListener) iter.next(); icud.execute(event, ..........); }
}finally{ event.getSession().getPersistenceContext().setFlushing(false); }
}

I appreciate any insight on this workaround.


S.Schnabl added a comment - 13/Sep/07 08:58 AM

Your workaround didn't work for me. You can test it with attached testcase and will see, that the exception will still occur. It think it's due the fact, that in
org.hibernate.event.def.AbstractFlushingEventListener inside method "flushEverythingToExecutions(FlushEvent event) " the following code

persistenceContext.setFlushing(true);
try { flushEntities(event); flushCollections(session); }
finally { persistenceContext.setFlushing(false); }

every time reset the flushing to TRUE on every every flush-event.


Ally Bongo added a comment - 14/Sep/07 05:44 AM

I'm getting this issue as well. I've updated the code in CollectionEntry to specifically check for the name of the collection I'm using, and ignoring postFlush if that's the case.

The collection I'm dealing with is immutable. The loadedPersister has a mutability check available to it. Would only performing the postFlush() on mutable collections be an option ie can you saftly ignore immutable collections in this process? eg include the first line:

if( loadedPersister != null || loadedPersister.isMutable(){
if ( isIgnore() ) { ignore = false; }
else if ( !isProcessed() ) { throw new AssertionFailure( "collection [" + collection.getRole() + "] was not processed by flush()" ); }
}


C. Delaloye added a comment - 19/Oct/07 07:19 AM

We have similar problems with other relation patterns. I've analysed the attached original TestCase (m:n relation, see testcase.zip) and came to these suggestions
which may solve the problem (background and details can be found in the attached file analyse_of_cdel.txt):

  • (Solution 1) Class CollectionEntry: set a default value for flag processed (see ####Comment 0.0)
  • Method StatefulPersistenceContext.addUninitializedCollection: one of these two points is sufficient to solve our problem
    -> (Solution 2) flag flushing has value false and we are in a flushing action: if this is
    a bug and if this bug is corrected, then the our bug is solved
    -> (Solution 3) flag processed could be set to true at this place because we are sure the collection
    was just read from the DB
  • (Solution 4) Constructor CollectionEntry (For collections just loaded from the database) (see ####Comment 26.2)
    Evaluation:
    Side effects for Solution 1 are difficult to evaluate: it affects all clients of the CollectionEntry class
    whose logic may be built on the asumption that the field has a false default value.
    Such a change affects CollectionEntry Items (i.e scope is limited).
    Side effects for Solution 2 are difficult to evaluate and are beyond the Collection scope.
    Solution 3 has the lowest side effect potential but with the risk that collections
    that are loaded in another way will still cause our bug.
    Solution 4: constuctor is also called by StatefulPersistenceContext.addInitializedCollection.
    Fixing here may have an undesired side effect.

C. Delaloye added a comment - 19/Oct/07 07:20 AM

Attachement to comment of cdel on Oct 19 2007


S.Schnabl added a comment - 23/Oct/07 08:43 AM

Thanks for the really good work ! I think it's now on the hibernate folk doing the next step to fix this problem....


C. Delaloye added a comment - 24/Oct/07 02:02 AM

In order to help the understanding of my previous comments, and I hope of the overall problem, I've prepared
two testCases which reproduce / illustrate the problem.
The attached HHH-2763_standalone_testcases.zip contains a "standalone" (no need of an app. server) Eclipse Project with a JUnit TestCase that reproduces the problem for these two mapping patterns:

  • testHibernateBugHHH2763Case1 (bidirectional many-to-many)
    this is the case initially described by the reporter
  • testHibernateBugHHH2763Case2 (one-to-many with many side having collections):
    this is a concrete case which is perhaps what Geoffrey De Smet has met but (to my mind) confusingly
    reported in it's comment of August 16th

Please have a look at the attached files bidirectional_many-to-many.gif and one-to-many_with_many_side_having_collections.gif which give
a graphical overview of the objects and relations.
These two cases uses a pre-update Listener which causes a Collection to be loaded during
the flush phase and then lead to the "was no processed by flush()" exception.

Entry point is Class: ch.rtc.infra.protok.client.InPro_ProtokHibernateEventListenerHibernateBugs_tc
Required to compile and run:

  • hibernate jars
  • junit.jar
  • commons-collections.jar, commons-lang.jar
  • hsqldb.jar

Another formulation of the bug would be:

the bug occurs when elements of a lazy collection (1) are loaded during a flush phase by a custom pre- or post- Event.
When these loaded elements (2) have mapped Collections (3), hibernate creates a CollectionEntry for each of them (them = mapped Collections).
The problem is that these created CollectionEntries are instantiated with flag processed=false and they have no chance
to have their "processed" flag set to true, as this only occurs in pre-flush phase and as this pre-flush phase is already done.
Consequently, the CollectionEntry.postflush() method which checks this "processed" flag will raise
the "was no processed by flush()" exception.

illustrated with Case2 (one-to-many with many side having collections):

(1) elements of a lazy collection: InPro_ParentWithLazyCollectionOfChildren.childrenWithCollections (many side)
(2) these loaded elements: instances of InPro_ChildWithCollections (many side)
(3) mapped Collections: collections defined on the many side: InPro_ChildWithCollections.collectionOfSimpleObjects and InPro_ChildWithCollections.collectionOfValues
Note that:

  • the problem is not dependent of the fact that the collection on the many side is lazy or not.
  • the problem also occurs if the many side has collections of values

Jakob Braeuchi added a comment - 06/Nov/07 10:01 AM

hi c. delaloye,

based on your testcase i found a solution that works without modification to the hibernate code.
i mark all CollectionEntries created by traversing the relationships as processed:

public boolean onPreUpdate(PreUpdateEvent event) {
Object entity = event.getEntity();
System.out.println("-- Entering pre-update for " +
entity.getClass().getName());

Object[] oldValues = event.getOldState();
String[] properties = event.getPersister().getPropertyNames();

// Iterate through all fields of the updated object
for (int ii = 0; ii < properties.length; ii++)
{
if (oldValues != null && oldValues[ii] instanceof Collection)
{
Collection col = (Collection) oldValues[ii];
// Hibernate will initialize(load) the n:side of m:n collection,
// but this will lead to following error later:
// AssertionFailure:collection [n:side] was not processed
// by flush()

// get CollectionEntries before traversing the relationship
List entriesBeforeLoad = new ArrayList();
entriesBeforeLoad.addAll(event.getSource().getPersistenceContext()
.getCollectionEntries().values());

// traverse the relationship and do some stuff
for (Iterator iter = col.iterator(); iter.hasNext()

{ Object element = (Object) iter.next(); System.out.println(element); }

// get CollectionEntries after traversing the relationship
List entriesAfterLoad = new ArrayList();
entriesAfterLoad.addAll(event.getSource().getPersistenceContext()
.getCollectionEntries().values());

// keep CollectionEntries created by traversing and mark them as processed.
entriesAfterLoad.removeAll(entriesBeforeLoad);
Iterator iter = entriesAfterLoad.iterator();
while (iter.hasNext())

{ CollectionEntry ce = (CollectionEntry) iter.next(); ce.setProcessed(true); }

}
}

System.out.println("-- Exiting pre-update for " +
entity.getClass().getName());
return true;
}

i'm not sure whether this causes any side-effects in your code. just give it a try.

greetings
jakob braeuchi


Adrian Pillinger added a comment - 18/Dec/07 10:36 AM

How close is this to completion? We have exactly the same error and it prevents us auditing changes to our data.
Thanks.


Darren Bell added a comment - 19/Dec/07 08:54 AM

This is a real show stopper for us. We need our data to be in many different databases (which is why we use hibernate) so we don't want to resort to triggers. This negates the usefulness of the listeners as auditing and history is one of its biggest uses.


Adrian Pillinger added a comment - 09/Jan/08 10:41 AM

My work around for this is to set the processed flag to true on all collections in the current persistence context within the listener. Since my listener is in the post insert/update/delete phase I hope that all collections have been processed correctly by now. Having said this, hibernate looks as though it will warn me about any side effect. I've looked at the hibernate code and the method Collections.prepareCollectionForUpdate(...) checks for collections being processed twice. Therefore if I set processed to true and it has not been processed yet hibernate will throw an exception.

My code to set the processed flag to true is as follows...

/**

  • Marks all collection entries in the current persistence context as processed
  • @param session The current session
    */
    private void markCollectionsAsProcessed(final Session session)
    {
    if (session instanceof SessionImplementor)
    Unknown macro: { SessionImplementor sessionImpl = (SessionImplementor)session; final PersistenceContext persistenceContext = sessionImpl.getPersistenceContext(); final Map collectionEntries = persistenceContext.getCollectionEntries(); final Collection values = collectionEntries.values(); for (Object obj }

    }


Herman Chung added a comment - 29/Feb/08 04:08 PM

For some reason I am getting the same error but I didn't even implement my own event listener. I am using JSF w/ Seam 2.0... anyone have any ideas?


Michael Newcomb added a comment - 27/Mar/08 11:20 AM

The problem is that AbstractFlushingEventListener.performExecutions() can result in new CollectionEntrys being created (because listeners access lazy collections, etc...). These CollectionEntry objects are technically created during a flush but not when the PersistenceConext.flushing attribute is set.

I looks safe to update AbstractFlushingEventListener.performExecutions() to:

session.getPersistenceContext().setFlushing(true);
try { session.getJDBCContext().getConnectionManager().flushBeginning(); // we need to lock the collection caches before // executing entity inserts/updates in order to // account for bidi associations session.getActionQueue().prepareActions(); session.getActionQueue().executeActions(); }
catch (HibernateException he) { log.error("Could not synchronize database state with session", he); throw he; }
finally { session.getPersistenceContext().setFlushing(false); session.getJDBCContext().getConnectionManager().flushEnding(); }

That should get rid of this issue, but I am have not analyzed everything that occurs in session.getActionQueue().executeActions() so there could be some side affect, but since the 'flushing' attribute is really only used for CollectionEntry objects, it seems safe.


Brooks Lyrette added a comment - 03/Jun/08 11:01 AM

I'm seeing the same issues when using Compass (2.0.0)

Compass uses event listeners to index data, and when it touches lazy relations this issue pops up. Is there any progress towards getting this fixed?


Denis VERGNES added a comment - 03/Jun/08 11:21 AM

I have also this issue using Compass 2.0.0 with Hibernate 3.2.6. A workaround is to remove lazy loading on your many-to-one relationship. It worked for me.


Brooks Lyrette added a comment - 03/Jun/08 11:46 AM

Thanks Denis. I am aware that this work around would fix the issue. I'd rather see it fixed in hibernate since it is a well know issue that should have been dealt with already.


Martin Backhaus added a comment - 03/Jun/08 12:42 PM

This problem i got too. My conclusion was, that you have not to change the state of the current object in the current session within events.
But if you want only use changed attributes - use the findDirty() (ore something called like this) to touch only changed (and loaded) attributes.
Iterate over dirty fields within the same session works fine for me. But i have to read in some cases all data -> i used another session.

If you would like to use another session, you should do the work within onPreUpdate() (not post), to fetch old data.


Dave Knipp added a comment - 12/Jun/08 12:57 PM

I'm encountering the same error, is anyone on the hibernate team working toward a solution to this?


Darren Bell added a comment - 16/Jun/08 03:34 AM

We'd like a resolution to this one. We've put a VERY horrible kludge in place to work around this, but it's very ugly and time consuming.


Adrian Pillinger added a comment - 17/Jun/08 02:43 AM

I've posted a thread on the forum to hopefully get some visibility on this issue.

http://forum.hibernate.org/viewtopic.php?t=987804


Gail Badner added a comment - 17/Jun/08 01:33 PM

Initializing a collection is not supported while the session is being flushed.

This would be difflicult to fix.

As Martin Backhaus mentioned above, the listener could use a different session to initialize the collection. He has provided a workaround in HHH-3317 that should also work here.

I realize this is an important issue and I plan to document the workaround and add it to the test suite.


Samppa Saarela added a comment - 09/Jan/09 10:17 AM

Hibernate-validator may also cause this error. For example

@entity class A { @ManyToOne(fetch=FetchType.LAZY) private A previous; @OneToMany(mappedBy="previous", fetch=FetchType.LAZY) // THIS CAUSES "collection was not processed by flush()" ERROR: @Size(min=0, max=1) private Set<A> next = new HashSet<A>(); @OneToMany(...,fetch=FetchType.LAZY) private Collection<Whatever> whatever; }

This example uses ManyToOne in order to avoid problems with OneToOne relation (forces eager load).

When flushing changes to an A#1 which has a non-initialized collection next containing A#2, @Size validator gets executed on pre flush phase and it initializes collection "next" that in turn loads entity A#2. Lazy collections of A#2 (whatever) don't get processed by flush and then raise this error.

There are at least two things to do here: 1) remove @Size wholly or 2) use @LazyCollection(LazyCollectionOption.EXTRA) option on the collection. But it raises also questions about usability of other, e.g. @Valid, validators on lazily intialized properties. Should validators be implemented so that they first check if a value has been initialized or can this be addressed within Hibernate somehow?


Eike Hirsch added a comment - 12/Jan/09 08:16 AM

Gail,

did you document the workaround already? Can you provide a link?


Gail Badner added a comment - 12/Jan/09 12:50 PM

Eike, no, I haven't documented this yet. I've been swamped with other things.


Eike Hirsch added a comment - 13/Jan/09 04:43 AM

Just for the record:

Envers is suffering from this bug as well.


Sanne Grinovero added a comment - 31/Jan/09 03:31 AM

Gail, I'm interested in the solution you propose; is using a different session still safe concerning transactions? is it possible to open a "child" session sharing the same transaction? I refer to repeatable-read, just to make sure the data you need is not gone already. (I'd love to do something like that to improve Hibernate Search's indexing performance so I'm interested in this solution, but in that case I need to keep the transaction alive for several minutes)


Steve Ebersole added a comment - 05/Mar/09 10:14 PM

Can y'all not validate in your listeners that the collection is initialized before doing stuff with it?

AFAICT, as Gail says, these scenarios all seem to boil down to listeners, interceptors, etc in some way adding stuff to the PC during flush which is a no-no.


Geoffrey De Smet added a comment - 06/Mar/09 06:35 AM

That's a pretty annoying no-no

What kind of solution would you recommend for this @AssertTrue?

@AssertTrue public boolean validateMomIsFemale() {
if (getMom() == null) { return true; }
return getMom().getSex() == Sex.FEMALE;
}

I suppose something like

@AssertTrue public boolean validateMomIsFemale() {
if (getMom() == null || !HibernateSpecificClassInMyDomainModel.isInitialized(getMom()) { return true; } }
return getMom().getSex() == Sex.FEMALE;
}

That HibernateSpecificClassInMyDomainModel is hibernate implementation detail creeping in my otherwise blissfully hibernate-unaware, jpa annoted domain classes?


Steve Ebersole added a comment - 06/Mar/09 09:05 AM

While I don't disagree with that, you are not considering the whole picture.

No idea what @AssertTrue, so I'll use the real JPA callbacks...

Imagine you have:
@PostUpdate
public void notifyMom { getMom().call(); getMom().setLastNotified( new Date() ); }

And for the sake of argument, lets just say mom is already associated to the session. What do you expect to have happen? And, what do you expect to have happen considering that this callback is already coming in the middle of a flush?

Now, if you want to discuss this further, there is a discussion about this on the hibernate-dev mailing list which is whether this should be discussed...


Geoffrey De Smet added a comment - 06/Mar/09 03:30 PM

AssertTrue is a hibernate-validator 3.3.x annotation.

Emmanuel talked on Dzone about the validation standardization and stated "Bean Validation should never go and traverse a lazy association and trigger the load of the association by side effect. That would be a very bad behavior. So, JPA has a way to put some limits in the object graph. The Bean Validation will actually validate."
So he's probably aware of the problems like this that AssertTrue causes.


nikita tovstoles added a comment - 16/Mar/09 04:05 PM

@Steve: So are you saying that EventListeners are not meant to traverse collections that were not instantiated within given Session?

What about this scenario:
Consider Child that belongs to Mom.children and Dad.children where both collections use DELETE_ORPHAN. if bidirectional mom.removeChild(child) is called dad.children 2nd level collection node will not be evicted on commit. In this case it's quite natural to implement a PreDeleteEventListener that will inspect Child being deleted and call dad.removeChild(thisChild) - causing the same assert problem. The other approach would be to introduce a service method to ensure removal child from any and all collections. But EventListener is a more natural approach as it is guaranteed to execute right before delete in question takes place. How else would you approach solving this problem?


Tim Stavenger added a comment - 20/May/09 11:06 AM

@Steve

I am encountering this same error; however, I have no custom listeners and am therefore not (willingly) iterating or initializing collections in a listener. We're only using the listeners that Hibernate 3.3.1.GA executes by default, so if Hibernate is doing this, it would lead me to believe that it is an error in Hibernate?

I've found that the change Michael Newcomb proposed on 27/Mar/08 11:20 AM (see his comment above) resolves my issue, but I don't know the impact of the change. It is in a class that appears you authored, so you may know? Michael proposes changing org.hibernate.event.def.AbstractFlushingEventListener#performExecutions(EventSource) to include setFlushing() calls:

protected void performExecutions(EventSource session) throws HibernateException {

log.trace("executing flush");

session.getPersistenceContext().setFlushing(true); // *************** new line

try { session.getJDBCContext().getConnectionManager().flushBeginning(); // we need to lock the collection caches before // executing entity inserts/updates in order to // account for bidi associations session.getActionQueue().prepareActions(); session.getActionQueue().executeActions(); }
catch (HibernateException he) { log.error("Could not synchronize database state with session", he); throw he; }
finally { session.getPersistenceContext().setFlushing(false); // ************** new line session.getJDBCContext().getConnectionManager().flushEnding(); }
}