[hibernate-dev] [ORM] Synchronization on AbstractLoadPlanBasedLoader

Gail Badner gbadner at redhat.com
Tue Dec 15 17:53:18 EST 2015


Looks like this is related to HHH-8679 and HHH-8939:

IIRC, the code in AbstractLoadPlanBasedLoader was copied from Loader.

Here's the commit that removed the comment you mention from Loader#
wrapResultSetIfEnabled and added a synchronized block to
retreiveColumnNameToIndexCache (HHH-8679):
https://github.com/hibernate/hibernate-orm/commit/71d5a746e331d805f336585f72723d46e746adf4

Here's the commit that removed the synchronized block from
Loader#retreiveColumnNameToIndexCache(HHH-8939):
https://github.com/hibernate/hibernate-orm/commit/af5804a49cc8a93a61733def6e1df779d6a2e02e

It looks like we didn't make the same changes when we moved to load plans.

On Tue, Dec 15, 2015 at 4:49 AM, Steve Ebersole <steve at hibernate.org> wrote:

> I did not add those comments; they were just in some code I copied over
> into that class.
>
> On Tue, Dec 15, 2015, 4:02 AM Sanne Grinovero <sanne at hibernate.org> wrote:
>
> > On 15 December 2015 at 01:46, Steve Ebersole <steve at hibernate.org>
> wrote:
> > > It's very possible that code comments may no longer be pertinent.
> >
> > Right, that's what I'm trying to figure out. Do you remember which
> > possible deadlock it might have referred to?
> >
> > >
> > > On Mon, Dec 14, 2015 at 10:26 AM Sanne Grinovero <sanne at hibernate.org>
> > > wrote:
> > >>
> > >> Hi all,
> > >> while reviewing an improvement by Stale about reducing
> > >> synchronization, I'm having the impression that the synchronization
> > >> could be completely removed.
> > >>
> > >> But there's a comment warning me against that, so for sake of safety
> > >> I'm merging the improvement without risking going too far:
> > >>
> > >>  // synchronized to avoid multi-thread access issues; defined as
> > >> method synch to avoid
> > >>  // potential deadlock issues due to nature of code.
> > >>
> > >> I tried to figure what "potential deadlock" it's referring to, but I'm
> > >> having the impression the comment might be outdated. So I've reduced
> > >> the contention to the only include the code block about which I'm not
> > >> confident.
> > >> By looking into git history, it seems the comment isn't related to any
> > >> specific fix but was included already when this class was first
> > >> created.
> > >>
> > >> Would someone be able to point out what is the issue this is
> protecting
> > >> against?
> > >>
> > >> That should allow us to provide an even better patch, although I'll
> > >> apply the safe one for now so to at least have the benefits already
> > >> when wrapping of result-sets is disabled.
> > >>
> > >> thanks,
> > >> Sanne
> > >> _______________________________________________
> > >> hibernate-dev mailing list
> > >> hibernate-dev at lists.jboss.org
> > >> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>


More information about the hibernate-dev mailing list