OK, I'll fix it for 5.2.11. Thanks, Gail
On Mon, Sep 11, 2017 at 12:20 PM, Steve Ebersole <st...@hibernate.org> wrote: > I'm fine with that. That method really does not fit with the paradigm > which needs that pulse. > > > On Mon, Sep 11, 2017 at 1:36 PM Sanne Grinovero <sa...@hibernate.org> > wrote: > > > On 11 September 2017 at 16:14, Guillaume Smet <guillaume.s...@gmail.com> > > wrote: > > > Hi! > > > > > > Any comment on this? > > > > > > After reading the javadoc of SharedSessionContractImplementor, I > think we > > > should probably just get rid of the `checkTransactionSynchStatus();` > call > > > in getInterceptor(). > > > > > > I don't think getInterceptor() should be responsible for joining the > > > transaction. > > > > > > What do you all think? > > > > +1 > > > > > > > > -- > > > Guillaume > > > > > > On Fri, Sep 8, 2017 at 6:26 PM, Guillaume Smet < > guillaume.s...@gmail.com > > > > > > wrote: > > > > > >> Hi, > > >> > > >> Note to Gail: this is potentially a blocking issue for 5.2.11 for the > > OGM > > >> upgrade (I thought it was a bug in OGM but apparently, it's an issue > > with > > >> ORM). > > >> > > >> In 5.2 SessionImpl, we now use getInterceptor() instead of accessing > the > > >> interceptor field directly because the field has been moved to > > >> AbstractSharedSessionContract. > > >> > > >> See for instance the change made here in afterTransactionCompletion(): > > >> https://github.com/hibernate/hibernate-orm/blame/master/ > > >> > > hibernate-core/src/main/java/org/hibernate/internal/ > SessionImpl.java#L2443 > > >> > > >> I think this is an issue as getInterceptor() is not a simple getter > but > > is: > > >> @Override > > >> public Interceptor getInterceptor() { > > >> checkTransactionSynchStatus(); > > >> return interceptor; > > >> } > > >> > > >> protected void checkTransactionSynchStatus() { > > >> pulseTransactionCoordinator(); > > >> delayedAfterCompletion(); > > >> } > > >> > > >> Thus calling the pulse() method of the TransactionCoordinator, > > triggering > > >> an implicit join whereas we're in the afterTransactionCompletion() > > phase. > > >> > > >> This is an issue for us as the pulse() method of our > > >> TransactionCoordinator creates Neo4j transactions so when the > > >> getInterceptor() method is called in afterTransactionCompletion(), we > > >> create a new Neo4j transaction. > > >> > > >> So 2 questions: > > >> - should we really call checkTransactionSynchStatus(); in > > >> getInterceptor()? If feels a bit weird. > > >> - if so, I think we should have a true protected getter (interceptor() > > >> following Steve's convention?) to avoid SessionImpl "pulsing" the > > >> transaction coordinator when accessing the interceptor. > > >> > > >> Thanks for your feedback! > > >> > > >> -- > > >> Guillaume > > >> > > > _______________________________________________ > > > hibernate-dev mailing list > > > hibernate-dev@lists.jboss.org > > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > _______________________________________________ > > hibernate-dev mailing list > > hibernate-dev@lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > _______________________________________________ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
