Re: 3.0.3: (BUG) Double-checked locking on enhancement

[Nick Westgate <ni...@key-planning.co.jp>]

Crikey. It's a textbook case.

The code in T4 is completely different because of Hivemind.
The comparable code syncs on the given specification, and
you should be able to do the same, namely patch it to:

public Class getEnhancedClass(IComponentSpecification specification,
         String className)
     {
         synchronized (specification)
         {
             Class result = getCachedClass(specification);

             if (result == null)
             {
                 result = getCachedClass(specification);
                 if (result == null)
                 {
                     result = constructComponentClass(specification,
className);
                     storeCachedClass(specification, result);
                 }
             }
         }

         return result;
     }

(The storeCachedClass() is ok because it uses a synchronized map.)

But there's another one in:
org.apache.tapestry.enhance.javassist.EnhancedClassFactory

     public CtClass getObjectType(String type)
     {
         CtClass result = getClassMapping().getType(type);

         if (result == null)
         {
             synchronized (this) {
                 result = getClassMapping().getType(type);

Cheers,
Nick.


Michael Prescott wrote:
> We're coming across an unusual exception that occurs when our
> application is under load.  Below is an example stack trace, although
> the problem can occur while enhancing any of a number of things, so it
> doesn't seem specific to one part of our application.  The root
> exception is always the same, however:
>  
>   javassist.CannotCompileException: duplicate method: <init>
>  
> Since it seemed to be related to concurrency, we were reading through
> the Tapestry classes along this stack, and found what looks like a
> threading problem in DefaultComponentClassEnhancer.java:
>  
>  public Class getEnhancedClass(IComponentSpecification specification,
> String className)
>     {
>         Class result = getCachedClass(specification);
>  
>         if (result == null)
>         {
>             synchronized (this)
>             {
>                 result = getCachedClass(specification);
>                 if (result == null)
>                 {
>                     result = constructComponentClass(specification,
> className);
>                     storeCachedClass(specification, result);
>                 }
>             }
>         }
>  
>         return result;
>     }
>  
> This is double-checked locking, which isn't safe given instruction
> reordering and thread-specific copies of variables, and all that
> multi-CPU magic.  This problem is occurring on an 8-CPU server, which
> I'm guessing is enough processors to expose the subtle ways in which
> double-checked locking can fail.
>  
> Does this ring any bells for anyone?  I realize this is a somewhat
> nebulous query.
>  
> Michael

---------------------------------------------------------------------
To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tapestry-user-help@jakarta.apache.org