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
