[hibernate-dev] Usage of static fields throughout Hibernate Core

[Gunnar Morling]

Or, yet better, change the method definition in the upstream component
(HCANN) so its accessible without jumping through reflective hoops.

--Gunnar


2016-04-28 12:57 GMT+02:00 Vlad Mihalcea <mihalcea.vlad at gmail.com>:

> Hi,
>
> I agree with.
>
> I'll open a new issue and link to this PR and your comment as well.
>
> Vlad
>
> On Thu, Apr 28, 2016 at 1:05 PM, Sanne Grinovero <sanne at hibernate.org>
> wrote:
>
> > Hi Vlad,
> >
> > there are several components which can safely use static fields, of
> > course as you say only if their state is not affected by which
> > SessionFactory is using them.
> >
> > So I would agree on treating those with care: it should always be a
> > warning sign during code reviews and warrant an in-depth check, but
> > ultimately it's a reasonable way to save some resources (both CPU and
> > memory) - especially when the same JVM runs multiple SessionFactory
> > instances.
> > Sure, savings might be small but when the component is clearly
> > stateless: why not.
> >
> > I would assume this is a technical detail which we can handle without
> > needing to make a blanket statement.
> >
> > +1 to avoid such tricks when it's not trivial to assert its safety,
> > yet there are valid use cases.
> >
> >
> > Looking at the specific case:
> > the usage of "private static Method memberMethod;" [1] seems dodgy
> indeed.
> > Synchronization is clearly not the best solution, as it would create a
> > contention point were there's no need to create one. It would even
> > contend across multiple SessionFactory instances which is worse than
> > usual :)
> >
> > Indeed the code is "racy" as different threads might need to repeat
> > the effort of finding the reference to "memberMethod", it would have
> > been better to make the field "volatile" to ensure different threads
> > would see it;
> > yet the code is not functionally incorrect as any thread invoking that
> > method will behave the same, and retrieve the right value.
> >
> > Better yet than volatile, make the "memberMethod" field Final, have it
> > initialized statically and move it to the top.. not sure why there are
> > fields hiding in the mid of a class :)
> >
> > BTW this is a good example of sane usage of static (final) fields: it
> > doesn't matter which SessionFactory is running or how it's configured,
> > but it's a good thing that different SessionFactory instances won't
> > have to repeat the initialization of that field over and over.
> >
> > Thanks,
> > Sanne
> >
> > 1 -
> >
> https://github.com/hibernate/hibernate-orm/pull/1198/files#diff-9f3d2f3cd0828afe20be0db68694f7a0L151
> >
> > On 28 April 2016 at 09:58, Vlad Mihalcea <mihalcea.vlad at gmail.com>
> wrote:
> > > Hi,
> > >
> > > While reviewing this PR:
> > >
> > > https://github.com/hibernate/hibernate-orm/pull/1198/files
> > >
> > > I realized that we have some static fields which might cause some
> > problems
> > > when we deploy two or more SessionFactories in the same container (e.g.
> > > Spring).
> > > The fix suggests that we should use synchronised, as indicated by
> Sonar,
> > > but I think we should avoid the static fields altogether.
> > >
> > > Every instance variable must be confined to an object that's confined
> to
> > > the same SessionFactory root object.
> > > Without static fields, we can make sure that nothing escapes a given
> > > SessionFactory.
> > >
> > > Let me know what you think.
> > >
> > > Vlad
> > > _______________________________________________
> > > 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
>