You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2012/01/31 04:02:24 UTC

DO NOT REPLY [Bug 52563] New: Incorrect behavior while checking the thread binding in DirContextURLStreamHandler

https://issues.apache.org/bugzilla/show_bug.cgi?id=52563

             Bug #: 52563
           Summary: Incorrect behavior while checking the thread binding
                    in DirContextURLStreamHandler
           Product: Tomcat 7
           Version: trunk
          Platform: PC
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Catalina
        AssignedTo: dev@tomcat.apache.org
        ReportedBy: xhhsld@gmail.com
    Classification: Unclassified


In the get method of DirContextURLStreamHandler class, it seems that binding
value with thread are not processed correctly. Now its value will be only
returned if no binding value on the current thread context classloader and it
has no parent classloader, think that the value will not be returned in most
scenarios.
Think that the code logic should be something like :
a. Check whether a value is binding on the current thread context classloader,
if does then return.
b. Check whether a value is binding with the current thread, if does then
return.
c. Check the classloader hierarchy to find a binding value.


public static DirContext get() { 

        DirContext result = null; 

        Thread currentThread = Thread.currentThread(); 
        ClassLoader currentCL = currentThread.getContextClassLoader(); 

        // Checking CL binding 
        result = clBindings.get(currentCL); 
        if (result != null) 
            return result; 

        // Checking thread biding 
        result = threadBindings.get(currentThread);

        // Checking parent CL binding 
        currentCL = currentCL.getParent(); 
        while (currentCL != null) { 
            result = clBindings.get(currentCL); 
            if (result != null) 
                return result; 
            currentCL = currentCL.getParent(); 
        } 

        if (result == null) 
            throw new IllegalStateException("Illegal class loader binding"); 

        return result; 

    }

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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


DO NOT REPLY [Bug 52563] Incorrect behavior while checking the thread binding in DirContextURLStreamHandler

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52563

--- Comment #3 from Ivan <xh...@gmail.com> 2012-02-05 07:47:26 UTC ---
I did not clarify it clearly, the issue occured in Geronimo, while I think that
it is better to do some changes on Tomcat side, I mean the get method.
On Tomcat side, in the startInternal method of StandardContext, the execution
order is something like :
a. resourcesStart() ---> Initial the resources variable
b. load.start() ---> Bind the resources with the current classloader
c. Some other initialization work, like servlet.onStartup, listener.start
But from the integration side, Geronimo has its own classloader creation and
destorying, and it did not has the WebAppLoader thing. 
>From the codes while invoking listenerStart/filterStart/loadOnStartup in
StandardContext, bindThread method will be invoked and actually I found that
Tomcat will also bind the context with the current method with the codes below
:
protected ClassLoader bindThread() {
......
DirContextURLStreamHandler.bindThread(getResources());
......
}
While that binding is 'always' ignored in the current
DirContextURLStreamHandler.get() method.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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


DO NOT REPLY [Bug 52563] Incorrect behavior while checking the thread binding in DirContextURLStreamHandler

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52563

Ivan <xh...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         OS/Version|                            |All

--- Comment #1 from Ivan <xh...@gmail.com> 2012-01-31 03:03:21 UTC ---
Had posted a message in the user list,
http://tomcat.10.n6.nabble.com/Correct-behavior-while-checking-the-thread-binding-in-DirContextURLStreamHandler-td4345124.html

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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


DO NOT REPLY [Bug 52563] Incorrect behavior while checking the thread binding in DirContextURLStreamHandler

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52563

--- Comment #5 from Konstantin Kolinko <kn...@gmail.com> 2012-02-05 19:21:58 UTC ---
Created attachment 28270
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=28270
2012-02-05_tc8_DirContextURLStreamHandler.patch

(In reply to comment #4)
> The current behaviour is intention. See r287546

So this behaviour was introduced 10 years ago and is well tested. Nobody ever
complained about seeing IllegalStateException("Illegal class loader binding")
failure enough to change the code.

(In reply to comment #0)
> Think that the code logic should be something like :
> a. Check whether a value is binding on the current thread context classloader,
> if does then return.
> b. Check whether a value is binding with the current thread, if does then
> return.
> c. Check the classloader hierarchy to find a binding value.
>

I am against a.,b.,c., because it changes current behaviour.

I would be OK with a.,c.,b.. With a,c,b it will have the same behavour as the
current code, but will handle an additional use case without throwing an ISE.

-> patch against trunk attached

But I would like to see a valid use case for this new feature. Some sample
code. Unused features are hard to maintain.


> Geronimo has its own classloader creation and
> destroying, and it did not has the WebAppLoader thing.

Maybe a bug is there? It is hard to understand the whole picture from your
short description in Comment 3.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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


DO NOT REPLY [Bug 52563] Incorrect behavior while checking the thread binding in DirContextURLStreamHandler

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52563

--- Comment #6 from Ivan <xh...@gmail.com> 2012-02-06 03:07:32 UTC ---
Thanks for your response. I copied the comment from the r287546
--->
Update the algortithm, weighting it towards trying the context CL association
  first (since that's what is used now in Catalina). In some very special
cases,
  the thread binding will be checked, as well as the parent classloader.
<---
>From the comment, I am thinking that it did not state that the initial purpose
is to check the classloader hierarchy firstly, and then the thread binding.
Also, from the code logic now, if it really means to check the classloader
hierarchy firstly, why not put those codes in the end of the get method ?
>From another side, think that in most application server, including Tomcat, the
context classloader should be the web application classloader, and it always
has parent classloader, which may be library classloader or server runtime
classloader, so the value binding on the thread will never have chance to be
used, as it always throw an IllegalStateException.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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


DO NOT REPLY [Bug 52563] Incorrect behavior while checking the thread binding in DirContextURLStreamHandler

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52563

--- Comment #2 from Christopher Schultz <ch...@christopherschultz.net> 2012-02-01 21:52:59 UTC ---
As mentioned in the user list, this code seems to work just fine -- it's just
slightly unclear while reading the code what is going on.

Is there a legitimate bug, here?

If so, please clearly state the steps-to-reproduce.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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


DO NOT REPLY [Bug 52563] Incorrect behavior while checking the thread binding in DirContextURLStreamHandler

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52563

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID

--- Comment #4 from Mark Thomas <ma...@apache.org> 2012-02-05 18:41:33 UTC ---
The current behaviour is intention. See r287546

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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