[jboss-as7-dev] Component upgrade of Javassist for AS7-5127...

[Scott Marlow]

On 07/06/2012 10:40 AM, Kabir Khan wrote:
>
> On 6 Jul 2012, at 13:57, Scott Marlow wrote:
>
>> We aren't using the latest Javassist on AS7 master.  Any arguments
>> against switching to the latest/shiniest 3.1.7.0-GA javassist?  That has
>> a fix for AS7-5127.
>>
>> Currently, the following AS7 modules have dependencies on Javassist:
>>
>> org.jboss.ws.native.jbossws-native-core
>> org.jboss.weld.core
>> org.jboss.as.weld
>> org.scannotation.scannotation
>> org.hibernate (envers also)
>>
>> Also, could someone (familiar with javassist) take a look at the code
>> change that AS7-5127 wants brought into AS7?  In the updated method,
>> parameter "java.lang.reflect.Method[] methods" is read outside of the
>> synchronized lock.
> I believe the check on line 53 needs to happen in a synchronized block as well to make sure it gets the latest, so rather than
> 	if (methods[index] == null) {
>   	    Method m1 = thisMethod == null ? null
>   	         : findMethod(self, thisMethod, desc);
>   	   Method m0 = findSuperMethod(self, superMethod, desc);
>   	   synchronized (methods) {
>                 if (methods[index] == null) {
>                   methods[index + 1] = m1;
>   	         methods[index] = m0;
>   	      }
>            }
>        }
>
> Something along the lines of this would be more correct
>
>     Method existing = null;
>     synchronized (methods) {
>         Method existing = methods[index];
>     }
>     if (existing == null) {
>   	    Method m1 = thisMethod == null ? null
>   	         : findMethod(self, thisMethod, desc);
>   	   Method m0 = findSuperMethod(self, superMethod, desc);
>   	   synchronized (methods) {
>                 if (methods[index] == null) {
>                   methods[index + 1] = m1;
>   	         methods[index] = m0;
>   	      }
>            }
>        }
>
>

That looks better but probably not that much more concurrent then the 
original code that performed both the read/write  in the same 
synchronization block.

Does anyone know if the array is longer than two elements (just curious 
if it contains just two entries or multiple pairs?)

>
>>
>> code change is here
>> https://source.jboss.org/browse/Javassist/trunk/src/main/javassist/util/proxy/RuntimeSupport.java?u=-1&r1=591&r2=637
>>
>> _______________________________________________
>> jboss-as7-dev mailing list
>> jboss-as7-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
>