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.v...@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@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
