tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r1172689 - /tomcat/trunk/java/org/apache/catalina/core/DefaultInstanceManager.java
Date Tue, 20 Sep 2011 11:34:37 GMT
2011/9/20 sebb <sebbaz@gmail.com>:
> On 19 September 2011 17:30,  <markt@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message