You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Simon Kitching <sk...@apache.org> on 2005/05/07 09:38:39 UTC

[logging] demonstration harness bug

Hi Robert,

I've found a small but critical problem with the "demonstration" harness
in class ClassLoaderRunner. When the test is *not* using a
context-classloader, the previous test's setting is not being cleared.

Without this patch, the following tests report "Log4JLogger does not
implement Log": 
  5, 6, 9, 10, 13, 14, 20, 21, 22, 24, 25, 26, 28, 29, 30, 32

With this patch, the following tests report this error:  
  17, 20, 21, 24, 28, 32

ClassLoaderRunner.java:
         if (setContextClassloader) {
             Thread.currentThread().setContextClassLoader(child);
+        } else {
+            Thread.currentThread().setContextClassLoader(parent);
         }


Regards,

Simon


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


Re: [logging] demonstration harness bug

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Sat, 2005-05-07 at 20:49 +1200, Simon Kitching wrote:
> On Sat, 2005-05-07 at 19:38 +1200, Simon Kitching wrote:
> > Hi Robert,
> > 
> > I've found a small but critical problem with the "demonstration" harness
> > in class ClassLoaderRunner. When the test is *not* using a
> > context-classloader, the previous test's setting is not being cleared.

darn :(

yes: you're right. that's a bug. well spotted 8-) 

i'll commit 

- robert



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


Re: [logging] demonstration harness bug

Posted by Simon Kitching <sk...@apache.org>.
On Sat, 2005-05-07 at 20:49 +1200, Simon Kitching wrote:
> On Sat, 2005-05-07 at 19:38 +1200, Simon Kitching wrote:
> > Hi Robert,
> > 
> > I've found a small but critical problem with the "demonstration" harness
> > in class ClassLoaderRunner. When the test is *not* using a
> > context-classloader, the previous test's setting is not being cleared.
> > 
> > Without this patch, the following tests report "Log4JLogger does not
> > implement Log": 
> >   5, 6, 9, 10, 13, 14, 20, 21, 22, 24, 25, 26, 28, 29, 30, 32
> > 
> > With this patch, the following tests report this error:  
> >   17, 20, 21, 24, 28, 32
> > 
> > ClassLoaderRunner.java:
> >          if (setContextClassloader) {
> >              Thread.currentThread().setContextClassLoader(child);
> > +        } else {
> > +            Thread.currentThread().setContextClassLoader(parent);
> >          }
> > 
> Hmm..if we do this instead
>   } else {
>     ClassLoader system = ClassLoader.getSystemClassLoader();
>     Thread.currentThread().setContextClassLoader(system);
>   }
> 
> Then cases 17 and 21 also pass. Interesting. I wonder what might
> possibly be causing this difference in behaviour!

Ok, I've looked into cases 17 and 21. The problem is that they are
simply invalid scenarios - ones that should never happen in real life.
So I'm not too worried about this.

Actually, I think *all* the scenarios that involve:
 * caller in child
 * AND child-first is true for child classloader
 * AND context classloader not set
are invalid. That doesn't make any sense to me.

However even with this setup, the test still works unless:
 * logging adapter class is in parent (ie commons-logging.jar) 
 * AND log class is in child (ie api or logging jar)

In the bad scenarios, child caller code loads LogFactory,
LogFactoryImpl, Log from the child classloader. Then LogFactoryImpl
loads the adapter from the contextClassLoader - which is the parent. The
adapter binds to Log via the parent too, with the result that the
logadapter implements Log@parent not Log@child, so the cast fails.

So I intend to ignore cases 17 and 21. Of course if we end up with a
solution to the general problem that fixes these too, that's a bonus.

Cheers,

Simon


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


Re: [logging] demonstration harness bug

Posted by Simon Kitching <sk...@apache.org>.
On Sat, 2005-05-07 at 19:38 +1200, Simon Kitching wrote:
> Hi Robert,
> 
> I've found a small but critical problem with the "demonstration" harness
> in class ClassLoaderRunner. When the test is *not* using a
> context-classloader, the previous test's setting is not being cleared.
> 
> Without this patch, the following tests report "Log4JLogger does not
> implement Log": 
>   5, 6, 9, 10, 13, 14, 20, 21, 22, 24, 25, 26, 28, 29, 30, 32
> 
> With this patch, the following tests report this error:  
>   17, 20, 21, 24, 28, 32
> 
> ClassLoaderRunner.java:
>          if (setContextClassloader) {
>              Thread.currentThread().setContextClassLoader(child);
> +        } else {
> +            Thread.currentThread().setContextClassLoader(parent);
>          }
> 
Hmm..if we do this instead
  } else {
    ClassLoader system = ClassLoader.getSystemClassLoader();
    Thread.currentThread().setContextClassLoader(system);
  }

Then cases 17 and 21 also pass. Interesting. I wonder what might
possibly be causing this difference in behaviour!

Note by the way that with either patch to the demonstration code, *every
single parent-first case* succeeds with the current JCL.

Regards,

Simon


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