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.v...@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 <sa...@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.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 > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
