Re: ***UNCHECKED*** Testcase for NPE problem in JSecurity (was: Intermediate release of JSecurity Plugin 0.3-SNAPSHOT)

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ***UNCHECKED*** Testcase for NPE problem in JSecurity (was: Intermediate release of JSecurity Plugin 0.3-SNAPSHOT)

Les Hazlewood
Administrator
------- Begin copy from previous emails so it can be archived on the dev
list -------------
Hello Les,
Hello Peter (I put you on cc in case you are interested),

I extracted the test-case. Please find attached the war-file.
It uses the in-memory DB so you can just throw it into you
tomcat web-app.

In our enviroment we can reproduce the problem as follow:

 - Tomcat 6.0.x  (on a Linux-System (tested with Debian and Gentoo), on
windows we can't reproduce)
 - Firefox 2     (Firefox 3 doesn't seem to get much of these NPEs)
 - You can login with the user "mos"  (password is also "mos")
 - Hit the "EDIT" link for username oder email field
 - Edit something and press "SAVE"
 - If the bug occur the app sent a 500 error and the page doesn't refresh
(progress bar stay on the page)

We log to stdout, so you can find some information in the catelina.out for
example.
Our JSECURITY-Logging looks like this:

---
[10:43:02] [DEBUG] [http-8080-2] [?:?] Principal: 'KeUser#1 [username:
mos]'. JSESSIONID-Cookie-Value:
["3616C0699338213544A383573602A896"]
Session values:
org.jsecurity.web.session.WebSession_INET_ADDRESS_SESSION_KEY=/127.0.0.1
org.codehaus.groovy.grails.FLASH_SCOPE=[:]
org.jsecurity.web.DefaultWebSecurityManager_PRINCIPALS_SESSION_KEY=[KeUser#1
[username: mos]]
org.jsecurity.web.DefaultWebSecurityManager_AUTHENTICATED_SESSION_KEY=true
---

The 'KeUser#1 [username: mos]' is a call to
'SecurityUtils.getSubject()?.principal'.
The other information are the cookie value and the session attributes.

In case of an error you see the following:

---
[10:43:02] [DEBUG] [http-8080-1] [?:?] Principal: 'null'.
JSESSIONID-Cookie-Value: ["3616C0699338213544A383573602A896"]
Session values:
org.jsecurity.web.session.WebSession_INET_ADDRESS_SESSION_KEY=/127.0.0.1
org.codehaus.groovy.grails.FLASH_SCOPE=[:]
org.jsecurity.web.DefaultWebSecurityManager_PRINCIPALS_SESSION_KEY=[KeUser#1
[username: mos]]
org.jsecurity.web.DefaultWebSecurityManager_AUTHENTICATED_SESSION_KEY=true
[10:43:02] [ERROR] [http-8080-1] [GrailsExceptionResolver.java:55]
java.lang.NullPointerException
       at
JsecurityGrailsPlugin.accessControlMethod(JsecurityGrailsPlugin.groovy:377)
---

Note:  The session information is valid and complete, but the JSecurity
subject is 'null'.
Could it be somehow related to the "Thread binding" of the subject?

Thanks a lot for your support

If you have any direct questions or hints, you are very welcome to contact
me via Skype:  "mo.scheele"

Cheers
mos

Peter's response:

OK, try the attached JAR file. You just need to replace the existing
one in "WEB-INF/lib". If it works, the problem is down to
ThreadContext using an InheritableThreadLocal rather than a standard
ThreadLocal. I don't know whether the InheritableThreadLocal is
important, but I suspect it's not safe in an environment where one
doesn't have control over the threads, for example in a servlet
container.

------- End copy from previous emails so it can be archived on the dev list
-------------

On Thu, Sep 4, 2008 at 11:02 AM, Peter Ledbrook <[hidden email]> wrote:

> > How did you narrow it down to that?  I'm just curious - could be useful
> in
> > my own debugging ;)
>
> Breakpoint in JSecurityFilters where it binds the security manager to
> the thread. I noticed that if there was a long enough pause, I was
> getting exceptions in other threads due to the ServletRequest not
> being bound to the thread. It seemed that as soon as execution
> switched to a different thread, the exception was thrown.

I could

> never work out what was really going on, but I suspected that the
> request was being unbound on one thread before it was being accessed
> on another. Anyway, I took a wild guess at the ThreadLocal and tried
> it with the parent class.
>
> > Also, I'm (somewhat) certain InheritableThreadLocal is not an invalid
> > choice, especially in security scenarios - if the parent thread has
> security
> > data bound to it, any child threads it spawns should also have the same
> > security data.  But there might be a problem if any of the child threads
> > modify the security data in any way - or if either the parent or child
> > thread clears out the ThreadContext before the others are done using it.
> > But according to the Servlet spec and JEE specs, requests and
> transactions
> > (respectively) have a 1:1 correspondence with a single thread, so this
> > parent/child thread corruption should never happen.
>
> I agree. I can't see why this would be a problem unless the
> ThreadContext is being manipulated by the parent of all the
> request-handling threads. Even then, it seems to me that only the
> initial values should be affected - once a request thread binds new
> values, it should be independent of any of the others. I don't get it,
> but the change seems to work for me.


Absolutely bizarre.  I have a strong suspicion this has to do with Tomcat's
thread pool behavior - maybe a single request thread is created, and others
are spawned based off of it for the pool.  Maybe the parent and its children
are in the same pool - if so, and either the parent or child(ren) call
ThreadContext.clear()  or ThreadContext.remove*, it would have an adverse
effect on either the parent or child(ren).  Again, this would only be the
case if my assumptions about the thread pool are correct.

In any event, a few months ago, I changed from ThreadLocal to
InheritableThreadLocal with the assumption that it was a good idea for
security systems as I described above.  If there are parent/child issues,
any child should receive a clone of the parent, ensuring modification of the
child's Map state does not affect the parent's and vice versa.

I've made that change to ThreadContext and just committed it.  Peter, could
you please update and test it out to ensure that it is a valid solution (I
still have deployment problems with that .war and can't test it).  Also MO,
if you check out jsecurity's trunk, you could build and deploy the created
jsecurity-jdk14.jar file and see yourself.

Please let me know what happens.

Thanks,

Les


By the way, the web app doesn't throw an NPE - it simply logs the fact

> that it would have done without an extra check added by mos :)
>
> > I'm also curious why you recommended trying the jsecurity-jdk14.jar - JDK
> > 1.4.2 has both the ThreadContext and InheritableThreadContext classes, so
> > there shouldn't be any difference between that .jar and jsecurity.jar
> > (1.5+).
>
> The plugin is packaged with the JDK1.4 version of the library, i.e.
> it's a like-for-like replacement. Nothing to do with potential
> differences between the JAR files.


Ah, ok, cool.  Thanks for the clarification.

>
>
> Cheers,
>
> Peter
>
> --
> Software Engineer
> G2One, Inc.
> http://www.g2one.com/
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ***UNCHECKED*** Testcase for NPE problem in JSecurity

Emmanuel Lécharny
Hi Les,

thanks for the feedback.

At some point, it would be better if we can redirect users having
problems to JIRA, as you can attach stack traces, logs, code, etc., and
have a better tracking system.

Btw, any estimate about when the code will be moved to ASF svn ?

Thanks !

Les Hazlewood wrote:

> ------- Begin copy from previous emails so it can be archived on the dev
> list -------------
> Hello Les,
> Hello Peter (I put you on cc in case you are interested),
>
> I extracted the test-case. Please find attached the war-file.
> It uses the in-memory DB so you can just throw it into you
> tomcat web-app.
>
> In our enviroment we can reproduce the problem as follow:
>
>  - Tomcat 6.0.x  (on a Linux-System (tested with Debian and Gentoo), on
> windows we can't reproduce)
>  - Firefox 2     (Firefox 3 doesn't seem to get much of these NPEs)
>  - You can login with the user "mos"  (password is also "mos")
>  - Hit the "EDIT" link for username oder email field
>  - Edit something and press "SAVE"
>  - If the bug occur the app sent a 500 error and the page doesn't refresh
> (progress bar stay on the page)
>
> We log to stdout, so you can find some information in the catelina.out for
> example.
> Our JSECURITY-Logging looks like this:
>
> ---
> [10:43:02] [DEBUG] [http-8080-2] [?:?] Principal: 'KeUser#1 [username:
> mos]'. JSESSIONID-Cookie-Value:
> ["3616C0699338213544A383573602A896"]
> Session values:
> org.jsecurity.web.session.WebSession_INET_ADDRESS_SESSION_KEY=/127.0.0.1
> org.codehaus.groovy.grails.FLASH_SCOPE=[:]
> org.jsecurity.web.DefaultWebSecurityManager_PRINCIPALS_SESSION_KEY=[KeUser#1
> [username: mos]]
> org.jsecurity.web.DefaultWebSecurityManager_AUTHENTICATED_SESSION_KEY=true
> ---
>
> The 'KeUser#1 [username: mos]' is a call to
> 'SecurityUtils.getSubject()?.principal'.
> The other information are the cookie value and the session attributes.
>
> In case of an error you see the following:
>
> ---
> [10:43:02] [DEBUG] [http-8080-1] [?:?] Principal: 'null'.
> JSESSIONID-Cookie-Value: ["3616C0699338213544A383573602A896"]
> Session values:
> org.jsecurity.web.session.WebSession_INET_ADDRESS_SESSION_KEY=/127.0.0.1
> org.codehaus.groovy.grails.FLASH_SCOPE=[:]
> org.jsecurity.web.DefaultWebSecurityManager_PRINCIPALS_SESSION_KEY=[KeUser#1
> [username: mos]]
> org.jsecurity.web.DefaultWebSecurityManager_AUTHENTICATED_SESSION_KEY=true
> [10:43:02] [ERROR] [http-8080-1] [GrailsExceptionResolver.java:55]
> java.lang.NullPointerException
>        at
> JsecurityGrailsPlugin.accessControlMethod(JsecurityGrailsPlugin.groovy:377)
> ---
>
> Note:  The session information is valid and complete, but the JSecurity
> subject is 'null'.
> Could it be somehow related to the "Thread binding" of the subject?
>
> Thanks a lot for your support
>
> If you have any direct questions or hints, you are very welcome to contact
> me via Skype:  "mo.scheele"
>
> Cheers
> mos
>
> Peter's response:
>
> OK, try the attached JAR file. You just need to replace the existing
> one in "WEB-INF/lib". If it works, the problem is down to
> ThreadContext using an InheritableThreadLocal rather than a standard
> ThreadLocal. I don't know whether the InheritableThreadLocal is
> important, but I suspect it's not safe in an environment where one
> doesn't have control over the threads, for example in a servlet
> container.
>
> ------- End copy from previous emails so it can be archived on the dev list
> -------------
>
> On Thu, Sep 4, 2008 at 11:02 AM, Peter Ledbrook <[hidden email]> wrote:
>
>  
>>> How did you narrow it down to that?  I'm just curious - could be useful
>>>      
>> in
>>    
>>> my own debugging ;)
>>>      
>> Breakpoint in JSecurityFilters where it binds the security manager to
>> the thread. I noticed that if there was a long enough pause, I was
>> getting exceptions in other threads due to the ServletRequest not
>> being bound to the thread. It seemed that as soon as execution
>> switched to a different thread, the exception was thrown.
>>    
>
> I could
>  
>> never work out what was really going on, but I suspected that the
>> request was being unbound on one thread before it was being accessed
>> on another. Anyway, I took a wild guess at the ThreadLocal and tried
>> it with the parent class.
>>
>>    
>>> Also, I'm (somewhat) certain InheritableThreadLocal is not an invalid
>>> choice, especially in security scenarios - if the parent thread has
>>>      
>> security
>>    
>>> data bound to it, any child threads it spawns should also have the same
>>> security data.  But there might be a problem if any of the child threads
>>> modify the security data in any way - or if either the parent or child
>>> thread clears out the ThreadContext before the others are done using it.
>>> But according to the Servlet spec and JEE specs, requests and
>>>      
>> transactions
>>    
>>> (respectively) have a 1:1 correspondence with a single thread, so this
>>> parent/child thread corruption should never happen.
>>>      
>> I agree. I can't see why this would be a problem unless the
>> ThreadContext is being manipulated by the parent of all the
>> request-handling threads. Even then, it seems to me that only the
>> initial values should be affected - once a request thread binds new
>> values, it should be independent of any of the others. I don't get it,
>> but the change seems to work for me.
>>    
>
>
> Absolutely bizarre.  I have a strong suspicion this has to do with Tomcat's
> thread pool behavior - maybe a single request thread is created, and others
> are spawned based off of it for the pool.  Maybe the parent and its children
> are in the same pool - if so, and either the parent or child(ren) call
> ThreadContext.clear()  or ThreadContext.remove*, it would have an adverse
> effect on either the parent or child(ren).  Again, this would only be the
> case if my assumptions about the thread pool are correct.
>
> In any event, a few months ago, I changed from ThreadLocal to
> InheritableThreadLocal with the assumption that it was a good idea for
> security systems as I described above.  If there are parent/child issues,
> any child should receive a clone of the parent, ensuring modification of the
> child's Map state does not affect the parent's and vice versa.
>
> I've made that change to ThreadContext and just committed it.  Peter, could
> you please update and test it out to ensure that it is a valid solution (I
> still have deployment problems with that .war and can't test it).  Also MO,
> if you check out jsecurity's trunk, you could build and deploy the created
> jsecurity-jdk14.jar file and see yourself.
>
> Please let me know what happens.
>
> Thanks,
>
> Les
>
>
> By the way, the web app doesn't throw an NPE - it simply logs the fact
>  
>> that it would have done without an extra check added by mos :)
>>
>>    
>>> I'm also curious why you recommended trying the jsecurity-jdk14.jar - JDK
>>> 1.4.2 has both the ThreadContext and InheritableThreadContext classes, so
>>> there shouldn't be any difference between that .jar and jsecurity.jar
>>> (1.5+).
>>>      
>> The plugin is packaged with the JDK1.4 version of the library, i.e.
>> it's a like-for-like replacement. Nothing to do with potential
>> differences between the JAR files.
>>    
>
>
> Ah, ok, cool.  Thanks for the clarification.
>
>  
>> Cheers,
>>
>> Peter
>>
>> --
>> Software Engineer
>> G2One, Inc.
>> http://www.g2one.com/
>>
>>    
>
>  


--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ***UNCHECKED*** Testcase for NPE problem in JSecurity

Les Hazlewood
Administrator
Hi Emmanuel,
I agree - this particular end-user messaged Peter and myself directly, and
although I almost always do what you recommend, for some reason I let this
one slip ;)  But yes, I'll try to remember that in the future.

I think we're very close to 0.9 final - just this issue that this user
reported and one other
(http://issues.jsecurity.org/browse/JSEC-125)<http://issues.jsecurity.org/browse/JSEC-125>.
 Once those are done, and 0.9 final is released, we can migrate over to the
ASF SVN asap.  Probably in a week or two at the most?

On Thu, Sep 4, 2008 at 11:57 AM, Emmanuel Lecharny <[hidden email]>wrote:

> Hi Les,
>
> thanks for the feedback.
>
> At some point, it would be better if we can redirect users having problems
> to JIRA, as you can attach stack traces, logs, code, etc., and have a better
> tracking system.
>
> Btw, any estimate about when the code will be moved to ASF svn ?
>
> Thanks !
>
> Les Hazlewood wrote:
>
>> ------- Begin copy from previous emails so it can be archived on the dev
>> list -------------
>> Hello Les,
>> Hello Peter (I put you on cc in case you are interested),
>>
>> I extracted the test-case. Please find attached the war-file.
>> It uses the in-memory DB so you can just throw it into you
>> tomcat web-app.
>>
>> In our enviroment we can reproduce the problem as follow:
>>
>>  - Tomcat 6.0.x  (on a Linux-System (tested with Debian and Gentoo), on
>> windows we can't reproduce)
>>  - Firefox 2     (Firefox 3 doesn't seem to get much of these NPEs)
>>  - You can login with the user "mos"  (password is also "mos")
>>  - Hit the "EDIT" link for username oder email field
>>  - Edit something and press "SAVE"
>>  - If the bug occur the app sent a 500 error and the page doesn't refresh
>> (progress bar stay on the page)
>>
>> We log to stdout, so you can find some information in the catelina.out for
>> example.
>> Our JSECURITY-Logging looks like this:
>>
>> ---
>> [10:43:02] [DEBUG] [http-8080-2] [?:?] Principal: 'KeUser#1 [username:
>> mos]'. JSESSIONID-Cookie-Value:
>> ["3616C0699338213544A383573602A896"]
>> Session values:
>> org.jsecurity.web.session.WebSession_INET_ADDRESS_SESSION_KEY=/127.0.0.1
>> org.codehaus.groovy.grails.FLASH_SCOPE=[:]
>>
>> org.jsecurity.web.DefaultWebSecurityManager_PRINCIPALS_SESSION_KEY=[KeUser#1
>> [username: mos]]
>> org.jsecurity.web.DefaultWebSecurityManager_AUTHENTICATED_SESSION_KEY=true
>> ---
>>
>> The 'KeUser#1 [username: mos]' is a call to
>> 'SecurityUtils.getSubject()?.principal'.
>> The other information are the cookie value and the session attributes.
>>
>> In case of an error you see the following:
>>
>> ---
>> [10:43:02] [DEBUG] [http-8080-1] [?:?] Principal: 'null'.
>> JSESSIONID-Cookie-Value: ["3616C0699338213544A383573602A896"]
>> Session values:
>> org.jsecurity.web.session.WebSession_INET_ADDRESS_SESSION_KEY=/127.0.0.1
>> org.codehaus.groovy.grails.FLASH_SCOPE=[:]
>>
>> org.jsecurity.web.DefaultWebSecurityManager_PRINCIPALS_SESSION_KEY=[KeUser#1
>> [username: mos]]
>> org.jsecurity.web.DefaultWebSecurityManager_AUTHENTICATED_SESSION_KEY=true
>> [10:43:02] [ERROR] [http-8080-1] [GrailsExceptionResolver.java:55]
>> java.lang.NullPointerException
>>       at
>>
>> JsecurityGrailsPlugin.accessControlMethod(JsecurityGrailsPlugin.groovy:377)
>> ---
>>
>> Note:  The session information is valid and complete, but the JSecurity
>> subject is 'null'.
>> Could it be somehow related to the "Thread binding" of the subject?
>>
>> Thanks a lot for your support
>>
>> If you have any direct questions or hints, you are very welcome to contact
>> me via Skype:  "mo.scheele"
>>
>> Cheers
>> mos
>>
>> Peter's response:
>>
>> OK, try the attached JAR file. You just need to replace the existing
>> one in "WEB-INF/lib". If it works, the problem is down to
>> ThreadContext using an InheritableThreadLocal rather than a standard
>> ThreadLocal. I don't know whether the InheritableThreadLocal is
>> important, but I suspect it's not safe in an environment where one
>> doesn't have control over the threads, for example in a servlet
>> container.
>>
>> ------- End copy from previous emails so it can be archived on the dev
>> list
>> -------------
>>
>> On Thu, Sep 4, 2008 at 11:02 AM, Peter Ledbrook <[hidden email]> wrote:
>>
>>
>>
>>> How did you narrow it down to that?  I'm just curious - could be useful
>>>>
>>>>
>>> in
>>>
>>>
>>>> my own debugging ;)
>>>>
>>>>
>>> Breakpoint in JSecurityFilters where it binds the security manager to
>>> the thread. I noticed that if there was a long enough pause, I was
>>> getting exceptions in other threads due to the ServletRequest not
>>> being bound to the thread. It seemed that as soon as execution
>>> switched to a different thread, the exception was thrown.
>>>
>>>
>>
>> I could
>>
>>
>>> never work out what was really going on, but I suspected that the
>>> request was being unbound on one thread before it was being accessed
>>> on another. Anyway, I took a wild guess at the ThreadLocal and tried
>>> it with the parent class.
>>>
>>>
>>>
>>>> Also, I'm (somewhat) certain InheritableThreadLocal is not an invalid
>>>> choice, especially in security scenarios - if the parent thread has
>>>>
>>>>
>>> security
>>>
>>>
>>>> data bound to it, any child threads it spawns should also have the same
>>>> security data.  But there might be a problem if any of the child threads
>>>> modify the security data in any way - or if either the parent or child
>>>> thread clears out the ThreadContext before the others are done using it.
>>>> But according to the Servlet spec and JEE specs, requests and
>>>>
>>>>
>>> transactions
>>>
>>>
>>>> (respectively) have a 1:1 correspondence with a single thread, so this
>>>> parent/child thread corruption should never happen.
>>>>
>>>>
>>> I agree. I can't see why this would be a problem unless the
>>> ThreadContext is being manipulated by the parent of all the
>>> request-handling threads. Even then, it seems to me that only the
>>> initial values should be affected - once a request thread binds new
>>> values, it should be independent of any of the others. I don't get it,
>>> but the change seems to work for me.
>>>
>>>
>>
>>
>> Absolutely bizarre.  I have a strong suspicion this has to do with
>> Tomcat's
>> thread pool behavior - maybe a single request thread is created, and
>> others
>> are spawned based off of it for the pool.  Maybe the parent and its
>> children
>> are in the same pool - if so, and either the parent or child(ren) call
>> ThreadContext.clear()  or ThreadContext.remove*, it would have an adverse
>> effect on either the parent or child(ren).  Again, this would only be the
>> case if my assumptions about the thread pool are correct.
>>
>> In any event, a few months ago, I changed from ThreadLocal to
>> InheritableThreadLocal with the assumption that it was a good idea for
>> security systems as I described above.  If there are parent/child issues,
>> any child should receive a clone of the parent, ensuring modification of
>> the
>> child's Map state does not affect the parent's and vice versa.
>>
>> I've made that change to ThreadContext and just committed it.  Peter,
>> could
>> you please update and test it out to ensure that it is a valid solution (I
>> still have deployment problems with that .war and can't test it).  Also
>> MO,
>> if you check out jsecurity's trunk, you could build and deploy the created
>> jsecurity-jdk14.jar file and see yourself.
>>
>> Please let me know what happens.
>>
>> Thanks,
>>
>> Les
>>
>>
>> By the way, the web app doesn't throw an NPE - it simply logs the fact
>>
>>
>>> that it would have done without an extra check added by mos :)
>>>
>>>
>>>
>>>> I'm also curious why you recommended trying the jsecurity-jdk14.jar -
>>>> JDK
>>>> 1.4.2 has both the ThreadContext and InheritableThreadContext classes,
>>>> so
>>>> there shouldn't be any difference between that .jar and jsecurity.jar
>>>> (1.5+).
>>>>
>>>>
>>> The plugin is packaged with the JDK1.4 version of the library, i.e.
>>> it's a like-for-like replacement. Nothing to do with potential
>>> differences between the JAR files.
>>>
>>>
>>
>>
>> Ah, ok, cool.  Thanks for the clarification.
>>
>>
>>
>>> Cheers,
>>>
>>> Peter
>>>
>>> --
>>> Software Engineer
>>> G2One, Inc.
>>> http://www.g2one.com/
>>>
>>>
>>>
>>
>>
>>
>
>
> --
> --
> cordialement, regards,
> Emmanuel Lécharny
> www.iktek.com
> directory.apache.org
>
>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ***UNCHECKED*** Testcase for NPE problem in JSecurity (was: Intermediate release of JSecurity Plugin 0.3-SNAPSHOT)

Peter Ledbrook
In reply to this post by Les Hazlewood
> I've made that change to ThreadContext and just committed it.  Peter, could
> you please update and test it out to ensure that it is a valid solution (I
> still have deployment problems with that .war and can't test it).  Also MO,
> if you check out jsecurity's trunk, you could build and deploy the created
> jsecurity-jdk14.jar file and see yourself.
> Please let me know what happens.
> Thanks,
> Les

Yup, that seems to work. Looking at the fix, that kind of makes sense.
If the ThreadContext is initialised by the parent thread of all the
request threads, then all the request threads will end up sharing the
same hash map - bad news! In fact, we should probably be creating a
new map every request and then removing it at the end. That's the
normal approach for safely working with thread pools, I think.

Cheers,

Peter

--
Software Engineer
G2One, Inc.
http://www.g2one.com/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ***UNCHECKED*** Testcase for NPE problem in JSecurity (was: Intermediate release of JSecurity Plugin 0.3-SNAPSHOT)

Les Hazlewood
Administrator
Do you think the ThreadContext.clear() method isn't sufficient enough?  Do
you still think it necessary to create a new instance?

On Thu, Sep 4, 2008 at 1:08 PM, Peter Ledbrook <[hidden email]> wrote:

> > I've made that change to ThreadContext and just committed it.  Peter,
> could
> > you please update and test it out to ensure that it is a valid solution
> (I
> > still have deployment problems with that .war and can't test it).  Also
> MO,
> > if you check out jsecurity's trunk, you could build and deploy the
> created
> > jsecurity-jdk14.jar file and see yourself.
> > Please let me know what happens.
> > Thanks,
> > Les
>
> Yup, that seems to work. Looking at the fix, that kind of makes sense.
> If the ThreadContext is initialised by the parent thread of all the
> request threads, then all the request threads will end up sharing the
> same hash map - bad news! In fact, we should probably be creating a
> new map every request and then removing it at the end. That's the
> normal approach for safely working with thread pools, I think.
>
> Cheers,
>
> Peter
>
> --
> Software Engineer
> G2One, Inc.
> http://www.g2one.com/
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ***UNCHECKED*** Testcase for NPE problem in JSecurity (was: Intermediate release of JSecurity Plugin 0.3-SNAPSHOT)

Peter Ledbrook
> Do you think the ThreadContext.clear() method isn't sufficient enough?  Do
> you still think it necessary to create a new instance?

Actually, that's probably OK. I was thinking of the unbind() methods
rather than the clear() method. I can't think of a reason why the
current setup won't work. The only advantage to creating a new map at
the start of the request is that it would work with an unmodified
InheritableThreadLocal, i.e. you wouldn't have to override
childValue() I think.

Anyway, I'm happy to leave it as it is now.

Cheers,

Peter

--
Software Engineer
G2One, Inc.
http://www.g2one.com/
Loading...