2011/9/20 sebb <seb...@gmail.com>: > On 19 September 2011 17:30, <ma...@apache.org> wrote: >> Author: markt >> Date: Mon Sep 19 16:30:36 2011 >> New Revision: 1172689 >> >> URL: http://svn.apache.org/viewvc?rev=1172689&view=rev >> Log: >> Fix threading issue with changing visibility of methods and fields >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java?rev=1172689&r1=1172688&r2=1172689&view=diff >> ============================================================================== >> --- tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java >> (original) >> +++ tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java >> Mon Sep 19 16:30:36 2011 >> @@ -187,9 +187,11 @@ public class DefaultInstanceManager impl >> if (entry.getType() == AnnotationCacheEntryType.POST_CONSTRUCT) { >> Method postConstruct = (Method) entry.getAccessibleObject(); >> boolean accessibility = postConstruct.isAccessible(); >> - postConstruct.setAccessible(true); >> - postConstruct.invoke(instance); >> - postConstruct.setAccessible(accessibility); >> + synchronized (postConstruct) { >> + postConstruct.setAccessible(true); >> + postConstruct.invoke(instance); >> + postConstruct.setAccessible(accessibility); >> + } > > Could skip the synch. block and two method calls if the field is > already accessible: > > if (accessibility) { > postConstruct.invoke(instance); > } else { > synchronized (postConstruct) { > postConstruct.setAccessible(true); > postConstruct.invoke(instance); > postConstruct.setAccessible(accessibility); > } > } > > Similarly for the other synch. blocks below. >
My understanding is that "isAccessible()" is false by default for a new instance of Method object (regardless of whether the method and class are public), and so your proposed optimization actually will never work. My thought is that setAccessible(true); can be called once - when the annotation is detected and placed into the cache. Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
