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

[Vlad Mihalcea]

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
>