Re: [hibernate-dev] HHH-5855 bug report and a possible fix

Hi guys I decided to spend some time to investigate the infamous HHH-5855 ( https://hibernate.atlassian.net/browse/HHH-5855 ) bug and this is my report. One of the first thing that I noticed is that Sets are fine, while this bug only replicates with bidirectional Bags. After some hours of debugging and logging (since debugging triggers collection initialization), I found the culprit. In the org.hibernate.type.TypeHelper.replace(Object[] original, Object[] target, Type[] types, SessionImplementor session, Object owner, Map copyCache) method, when copying the cached entity state (which contains the newly added child entity along with its identifier) onto the original collection: copied[i] = types[i].replace( original[i], target[i], session, owner, copyCache ); inside the org.hibernate.type.CollectionType.replace(Object[] original, Object[] target, Type[] types, SessionImplementor session, Object owner, Map copyCache) method there is this check: if ( !Hibernate.isInitialized( original ) ) { return target; } For Sets, the collection is always initialized because of this line inside the add() method of the org.hibernate.collection.internal.PersistentSet: final Boolean exists = isOperationQueueEnabled() ? readElementExistence( value ) : null; Because of the the readElementExistence( value ) call the Set is always initialized and upon triggering the flush, the newly added Entity being already managed it will be left alone. For PersistentList the aforementioned check is false and the replace never occurs, hence the transient entity lingers in the persistence context and the flush will trigger another insert, so we get duplicates. To make the Bag behave like the Set, all we need to do is to change the add method like this: public boolean add(Object object) { initialize(false); if ( !isOperationQueueEnabled() ) { write(); return bag.add( object ); } else { queueOperation( new SimpleAdd( object ) ); return true; } } but then four tests will fail: org.hibernate.test.legacy.MasterDetailTest > testQueuedBagAdds FAILED java.lang.AssertionError at MasterDetailTest.java:1068 org.hibernate.test.unionsubclass.UnionSubclassTest > testUnionSubclass FAILED org.hibernate.ObjectDeletedException at UnionSubclassTest.java:364 org.hibernate.test.version.VersionTest > testCollectionNoVersion FAILED java.lang.AssertionError at VersionTest.java:118 org.hibernate.test.version.VersionTest > testCollectionVersion FAILED java.lang.AssertionError at VersionTest.java:79 3 of them fail because we expect the List not to be initialized and the UnionSubclassTest fails for a doubtful reason (we attempt to delete an entity which is still referenced). Basically, such a change will finally fix this issue and the Sets and Lists will behave consistently. Since you know the reasons behind the difference in how Sets and Lists are initialized, we need to discuss if this change is appropriate or we should address this issue differently. I have a branch on my fork with a test that replicates this issue and that the proposed change manages to fix it: https://github.com/vladmihalcea/hibernate-orm/tree/feature/hhh5855 Let me know what you think and let's discuss it further. Vlad _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev