camel-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christoph Läubrich (JIRA) <j...@apache.org>
Subject [jira] [Commented] (CAMEL-10456) Camel leaks TCCL
Date Thu, 10 Nov 2016 10:05:58 GMT

    [ https://issues.apache.org/jira/browse/CAMEL-10456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15653643#comment-15653643
] 

Christoph Läubrich commented on CAMEL-10456:
--------------------------------------------

The pattern in Object-Helper assumes that the ttcl of a thead is never null, that is the problem
(why this was changed from bug to imrpovement?) the pattern itself is not a problem.
We see this in Karaf+Camel+Blueprint using a CXF component. Most of the web-qtp-threads are
infected by a BundleDelegationClassloader that referneces a bundle that was previously uninstalled.
So now all attempts to use the tccl in such a thread (it seems CXF tries to find Serviceloaders)
are resulting in a RuntimeException because the findResource method throw an IllegalStateException:
Bundle uninstalled.
This can be fixed by restarting the container until the bundle that is left in the TCCL is
uninstalled and reinstalled again. Beside this it prevents the old Bundle (and its classes)
from beeing garbage-collected.

I created a test-case that shows the problem:
{code}public class TCCLTest {

    private final ExecutorService executor = Executors.newFixedThreadPool(1, new ThreadFactory()
{

                                               @Override
                                               public Thread newThread(Runnable r) {
                                                   Thread thread = new Thread(r);
                                                   //Explicitly set it to null so it does
not use the one from the parent thread
                                                   thread.setContextClassLoader(null);
                                                   return thread;
                                               }
                                           });

    private final class MyTestClassLoaderExtension extends ClassLoader {
    }

    @Test
    public void testResetTCCL() throws Exception {
        final Callable<ClassLoader> tccfetcher = new Callable<ClassLoader>() {

            @Override
            public ClassLoader call() throws Exception {
                return Thread.currentThread().getContextClassLoader();
            }
        };
        final MyTestClassLoaderExtension classloader1 = new MyTestClassLoaderExtension();
        Assert.assertNull("TCCL was not null initially", executor.submit(tccfetcher).get());
        ClassLoader resultOfObjectHelper = executor.submit(new Callable<ClassLoader>()
{

            @Override
            public ClassLoader call() throws Exception {
                return (ClassLoader) ObjectHelper.callWithTCCL(tccfetcher, classloader1);
            }

        }).get();
        Assert.assertEquals("TCCL was not the one given to ObjectHelper!", classloader1, resultOfObjectHelper);
        ClassLoader classLoaderAfterObjecthelperCallInThreadPool = executor.submit(tccfetcher).get();
        Assert.assertNull("TCCL was not reset to null but is now " + classLoaderAfterObjecthelperCallInThreadPool,
classLoaderAfterObjecthelperCallInThreadPool);
    }
}
{code}

Proposed patch:
 {code}/**
     * Calling the Callable with the setting of TCCL with a given classloader.
     * 
     * @param call
     *            the Callable instance
     * @param classloader
     *            the class loader
     * @return the result of Callable return
     */
    public static Object callWithTCCL(Callable<?> call, ClassLoader classloader) throws
Exception {
        ClassLoader tccl = Thread.currentThread().getContextClassLoader();
        try {
            if (classloader != null) {
                Thread.currentThread().setContextClassLoader(classloader);
            }
            return call.call();
        } finally {
            if (classloader != null) { /* old code:  if (tccl != null) { */
                Thread.currentThread().setContextClassLoader(tccl);
            }
        }
    }{code}Since it is not clear what pice of code has leaked the TCCL (I just fund this bug
in ObjectHelper yet that seems to be related Exchanges) it might be good to check if other
camel code can use the ObjectHelper (if fixed) to prevent similar bugs elsewhere (that might
be a seperate improvement).

Beside this, the code in ObjectHelper can be made generic to prevent needs for casting like
this:
{code} public static <T> T callWithTCCL(Callable<T> call, ClassLoader classloader)
throws Exception {{code}and{code} public static <T> T callWithTCCL(Callable<T>
call, Exchange exchange) throws Exception {{code}
    

> Camel leaks TCCL
> ----------------
>
>                 Key: CAMEL-10456
>                 URL: https://issues.apache.org/jira/browse/CAMEL-10456
>             Project: Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.18.0
>            Reporter: Christoph Läubrich
>             Fix For: Future
>
>
> Camel leaks ThreadContextClassLoader instances at least in the following place:
> camel-core: https://github.com/apache/camel/blob/4f9448d83cc21a348f92cca961907b0f72d9db79/camel-core/src/main/java/org/apache/camel/util/ObjectHelper.java#L1913
> As mentioned in the JavaDoc of Thread.getContextClassLoader() returning "null indicating
the system class loader (or, failing that, the bootstrap class loader)", se same applies to
Thread.setContextClassLoader(...)
> The code only reset the TCCL if the returned value from Thread.currentThread().getContextClassLoader()
was != null. So if in a thread without a TCCL set (and thus returning null) these methods
set a new CCL but later do not reset these to the original null value.
> This leads to Threads (e.g. when taking reused from a pool) having a classloader that
will never gets reset and thus can't be garbage collected or even lead to strange behaviour
because if other code that uses the TCCL-mechanism can try to load classes or resources from
this loader later on.
> I found that https://github.com/apache/camel/blob/master/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsProducer.java#L85
uses a similar pattern, only resetting the TCCL if the *new* TCCL != null so maybe the code
in ObjectHelper was meant to check for classloader != null instead of tccl !=null
> The fix should also include making sure this pattern is not used in other camel-components
or even to use the ObjectHelper Method consistently, currently it seems may components implement
their owh TCCL-handling.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message