You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomcat.apache.org by Ivan <xh...@gmail.com> on 2012/01/28 05:40:06 UTC
Correct behavior while checking the thread binding in
DirContextURLStreamHandler ?
Hi, devs, in the get method of DirContextURLStreamHandler class, I saw the
codes below :
--->
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);
<-------------- Here, the value from threadBindings is always ignored ? is
there something like "if (result != null) return result;" required there ?
// 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;
}
--
Ivan
Re: Correct behavior while checking the thread binding in DirContextURLStreamHandler
?
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Pid,
> On 01/02/2012 11:50, Ivan wrote:
>> 2012/2/1 Christopher Schultz <ch...@christopherschultz.net>
>>
>> Of course, you still need to check for null after the loop, so
>> it's not like the change really affects anything other than minor
>> readability.
>
> No sure whether I understood you clearly, if a context is binding
> on the current thread, and current context classloader has parent
> classloader, current get method will throw an
> IllegalStateException. So my question is that, in this scenario,
> should the context binded on the thread be ignored ? I did not find
> too many comments on the svn log, while I guess that the logic may
> be : a. Check whether there is a context binding on the current
> context classloader, if does, return it. b. Check whether there is
> a context binding on the current thread, if does, return it. c.
> Check whether there is a context binding on the classloader tree of
> the current context classloader, if does return it. d. Throw an
> IllegalStateException.
I still can't figure out if you think there's an actual bug here.
Is the problem that you object to illogical code? Is there a branch
that does not need to exist because you can prove that a certain
situation will never arise?
I find this kind of thing sometimes when using FindBugs: fb will tell
me that a certain local variable being checked for null somewhere
results in dead-code in the body of the if, like this:
if(null != conn) { conn.close(); }
fb says that "conn" can never be non-null here, so the code is dead. I
always tell fb to ignore that because I don't want to modify the code
sometime in the future such that "conn" *might* be null and forget to
add the null-check (or conn.close) back into the code.
Defensive programming sometimes results in illogical code.
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk8qptMACgkQ9CaO5/Lv0PCV+ACeIx3y69FvjIbaasS2seLtDqm4
lEUAnipaSpnrn1Figs0TQ9ucS+FN3+5I
=nWlV
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Correct behavior while checking the thread binding in DirContextURLStreamHandler
?
Posted by Pid <pi...@pidster.com>.
On 01/02/2012 11:50, Ivan wrote:
Here's what the reply should have looked like, without the top-posting.
> 2012/2/1 Christopher Schultz <ch...@christopherschultz.net>
>
> Ivan,
>
> On 1/27/12 11:40 PM, Ivan wrote:
>>>> if (result != null) return result;
>>>>
>>>> // Checking thread biding result =
>>>> threadBindings.get(currentThread); <-------------- Here, the value
>>>> from threadBindings is always ignored ? is there something like "if
>>>> (result != null) return result;" required there ?
>>>>
>>>> // 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;
>
> That does look fishy.
>
> "result" will be ignored if currentCL.getParent returns non-null. If
> currentCL.getParent returns null, then the method throws an exception.
> It looks like this could be re-written in a more straightforward way
> like this:
>
> currentCL = currentCL.getParent();
> if(null == result && null == currentCL)
> throw new IllegalArgumentException(...);
>
> while(currentCL != null)
> // continue
>
> Of course, you still need to check for null after the loop, so it's
> not like the change really affects anything other than minor readability.
No sure whether I understood you clearly, if a context is binding on the
current thread, and current context classloader has parent classloader,
current get method will throw an IllegalStateException. So my question is
that, in this scenario, should the context binded on the thread be ignored ?
I did not find too many comments on the svn log, while I guess that the
logic may be :
a. Check whether there is a context binding on the current context
classloader, if does, return it.
b. Check whether there is a context binding on the current thread, if does,
return it.
c. Check whether there is a context binding on the classloader tree of the
current context classloader, if does return it.
d. Throw an IllegalStateException.
>
> -chris
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: users-help@tomcat.apache.org
>>
>>
>
>
--
[key:62590808]
Re: Correct behavior while checking the thread binding in
DirContextURLStreamHandler ?
Posted by Ivan <xh...@gmail.com>.
No sure whether I understood you clearly, if a context is binding on the
current thread, and current context classloader has parent classloader,
current get method will throw an IllegalStateException. So my question is
that, in this scenario, should the context binded on the thread be ignored ?
I did not find too many comments on the svn log, while I guess that the
logic may be :
a. Check whether there is a context binding on the current context
classloader, if does, return it.
b. Check whether there is a context binding on the current thread, if does,
return it.
c. Check whether there is a context binding on the classloader tree of the
current context classloader, if does return it.
d. Throw an IllegalStateException.
2012/2/1 Christopher Schultz <ch...@christopherschultz.net>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Ivan,
>
> On 1/27/12 11:40 PM, Ivan wrote:
> > if (result != null) return result;
> >
> > // Checking thread biding result =
> > threadBindings.get(currentThread); <-------------- Here, the value
> > from threadBindings is always ignored ? is there something like "if
> > (result != null) return result;" required there ?
> >
> > // 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;
>
> That does look fishy.
>
> "result" will be ignored if currentCL.getParent returns non-null. If
> currentCL.getParent returns null, then the method throws an exception.
> It looks like this could be re-written in a more straightforward way
> like this:
>
> currentCL = currentCL.getParent();
> if(null == result && null == currentCL)
> throw new IllegalArgumentException(...);
>
> while(currentCL != null)
> // continue
>
> Of course, you still need to check for null after the loop, so it's
> not like the change really affects anything other than minor readability.
>
> - -chris
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
> Comment: GPGTools - http://gpgtools.org
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAk8oUbUACgkQ9CaO5/Lv0PDGvwCgkELv0wJaVlzVxJc+UkUxi+Vx
> 9vAAnAgQYg0loutHFkxYxEzoJFqZYb3I
> =e8sd
> -----END PGP SIGNATURE-----
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
>
--
Ivan
Re: Correct behavior while checking the thread binding in DirContextURLStreamHandler
?
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Ivan,
On 1/27/12 11:40 PM, Ivan wrote:
> if (result != null) return result;
>
> // Checking thread biding result =
> threadBindings.get(currentThread); <-------------- Here, the value
> from threadBindings is always ignored ? is there something like "if
> (result != null) return result;" required there ?
>
> // 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;
That does look fishy.
"result" will be ignored if currentCL.getParent returns non-null. If
currentCL.getParent returns null, then the method throws an exception.
It looks like this could be re-written in a more straightforward way
like this:
currentCL = currentCL.getParent();
if(null == result && null == currentCL)
throw new IllegalArgumentException(...);
while(currentCL != null)
// continue
Of course, you still need to check for null after the loop, so it's
not like the change really affects anything other than minor readability.
- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk8oUbUACgkQ9CaO5/Lv0PDGvwCgkELv0wJaVlzVxJc+UkUxi+Vx
9vAAnAgQYg0loutHFkxYxEzoJFqZYb3I
=e8sd
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Correct behavior while checking the thread binding in
DirContextURLStreamHandler ?
Posted by Ivan <xh...@gmail.com>.
So is the current behavior by design, or it is a defect ?
2012/1/28 Ivan <xh...@gmail.com>
> Yes, but guess that getParent() will not be null in most scenarios :-) And
> if the thread binding value is expected to be returned while no classloader
> binding, think the codes should be in the end of the method.
>
> 2012/1/28 Caldarale, Charles R <Ch...@unisys.com>
>
> > From: Ivan [mailto:xhhsld@gmail.com]
>> > Subject: Correct behavior while checking the thread binding in
>> DirContextURLStreamHandler ?
>>
>> > result = threadBindings.get(currentThread);
>> > <-------------- Here, the value from threadBindings is always ignored ?
>>
>> No, it's not always ignored. If
>> currentThread.getContextClassLoader().getParent() is null, the value from
>> ghreadBindings.get() will be returned. Of course, I don't know if the
>> getParent() call can ever return null; that might well depend on actions
>> taken inside the webapp application code.
>>
>> - Chuck
>>
>>
>> THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY
>> MATERIAL and is thus for use only by the intended recipient. If you
>> received this in error, please contact the sender and delete the e-mail and
>> its attachments from all computers.
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: users-help@tomcat.apache.org
>>
>>
>
>
> --
> Ivan
>
--
Ivan
Re: Correct behavior while checking the thread binding in
DirContextURLStreamHandler ?
Posted by Ivan <xh...@gmail.com>.
Yes, but guess that getParent() will not be null in most scenarios :-) And
if the thread binding value is expected to be returned while no classloader
binding, think the codes should be in the end of the method.
2012/1/28 Caldarale, Charles R <Ch...@unisys.com>
> > From: Ivan [mailto:xhhsld@gmail.com]
> > Subject: Correct behavior while checking the thread binding in
> DirContextURLStreamHandler ?
>
> > result = threadBindings.get(currentThread);
> > <-------------- Here, the value from threadBindings is always ignored ?
>
> No, it's not always ignored. If
> currentThread.getContextClassLoader().getParent() is null, the value from
> ghreadBindings.get() will be returned. Of course, I don't know if the
> getParent() call can ever return null; that might well depend on actions
> taken inside the webapp application code.
>
> - Chuck
>
>
> THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY
> MATERIAL and is thus for use only by the intended recipient. If you
> received this in error, please contact the sender and delete the e-mail and
> its attachments from all computers.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
>
--
Ivan
RE: Correct behavior while checking the thread binding in
DirContextURLStreamHandler ?
Posted by "Caldarale, Charles R" <Ch...@unisys.com>.
> From: Ivan [mailto:xhhsld@gmail.com]
> Subject: Correct behavior while checking the thread binding in DirContextURLStreamHandler ?
> result = threadBindings.get(currentThread);
> <-------------- Here, the value from threadBindings is always ignored ?
No, it's not always ignored. If currentThread.getContextClassLoader().getParent() is null, the value from ghreadBindings.get() will be returned. Of course, I don't know if the getParent() call can ever return null; that might well depend on actions taken inside the webapp application code.
- Chuck
THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers.
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org