EJB-thread (which becomes pooled) inherits store from current web-thread

[daniell]

I have a webapplication running on Widlfly 8.0.0.Final. To secure access to the webinterface we utilize a third-party-library (Apache Shiro). This library stores it's context information (logged-in user, ...) in an InheritableThreadLocal variable, which is added and removed by a Servlet-Filter.

If I start the application-server and enter the webinterface, the filter adds some information to this InheritableThreadLocal variable. At some point the application calls an @Asynchronous method of a SFSB.

It seems that the following happens: since the thread-pool is empty at this moment, the application-server creates a new thread to execute the asynchronous EJB-method. Since there is an InheritableThreadLocal which contains some data, this data is inherited to this newly created thread. After finishing the method, the thread becomes pooled and may be reused later. After the web-request finishes, the servlet-filter removes the information in the InheritableThreadLocal of the current thread, but obviously does not touch the spawned (pooled) thread - leading to leak some information to the pooled thread.

If I call some EJB methods afterwards (e.g. via the remote-interface), I can read the previously stored information if the thread which has been created before is used.

I haven't expected this since I would suppose that the threads which are in the pool are "clean" and do not depend on the thread which (randomly) initiate their creation because of first usage.

[jameslivingston]

Daniel Lechner wrote:

To secure access to the webinterface we utilize a third-party-library (Apache Shiro). This library stores it's context information (logged-in user, ...) in an InheritableThreadLocal variable, which is added and removed by a Servlet-Filter.

I would say that this is the problem - InheritableThreadLocal is obviously going to cause problems when other code (such as the container) creates a new thread. I wonder why Shiro uses that and not a normal ThreadLocal.

I haven't expected this since I would suppose that the threads which are in the pool are "clean" and do not depend on the thread which (randomly) initiate their creation because of first usage.

The only way I can think of making that safe in the presence of InheritableThreadLocals would be to have a special thread whose sole job is to creates the other thread to prevent any InheritableThreadLocals from being set, and all thread creating is runs as a task on it.

I do not believe there is any way for the container to clear any InheritableThreadLocals that it does not know about.

[jameslivingston]

A post from Shrio's developers in 2008, http://shiro-developer.582600.n2.nabble.com/Re-UNCHECKED-Testcase-for-NPE-problem-in-JSecurity-was-Intermediate-release-…, seems to indicate they weren't sure if it would be problematic. They mention this scenario as one of the potential issues.

[daniell]

James Livingston schrieb:


Daniel Lechner wrote:

To secure access to the webinterface we utilize a third-party-library (Apache Shiro). This library stores it's context information (logged-in user, ...) in an InheritableThreadLocal variable, which is added and removed by a Servlet-Filter.

I would say that this is the problem - InheritableThreadLocal is obviously going to cause problems when other code (such as the container) creates a new thread. I wonder why Shiro uses that and not a normal ThreadLocal.

Exactly. I've found no hint why they use InheritableThreadLocal. As far as I have seen, they use it since Revision 710574 (August 2007). See this diff.

I haven't expected this since I would suppose that the threads which are in the pool are "clean" and do not depend on the thread which (randomly) initiate their creation because of first usage.
The only way I can think of making that safe in the presence of InheritableThreadLocals would be to have a special thread whose sole job is to creates the other thread to prevent any InheritableThreadLocals from being set, and all thread creating is runs as a task on it.

I do not believe there is any way for the container to clear any InheritableThreadLocals that it does not know about.

That's the question. Despite from this issue, I think the container should be able to spawn "clean" threads if they are potentially used afterwards. As already written it sounds unsafe to me if the newly created threads depend on the context where they are randomly created. This leads to random errors which are hard to find (like mine ).

I have no idea how this can be archived. Your suggestion with the special thread sounds reasonable and is one possible solution.

[daniell]

Grrr - I hate this forum software. It is hard to paste or edit anything in the rich editor. Despite everything looks fine in the editor, but after submitting the message the post looks ugly

[daniell]

Related question at Shrio: http://shiro-user.582556.n2.nabble.com/Inheritance-of-Security-Context-causes-problems-in-EJB-container-td7579859.html

[jaikiran]

JBoss Threads (which is what backs the container thread pools) has something called as a ThreadLocalResetter jboss-threads/src/main/java/org/jboss/threads/ThreadLocalResetter.java at master · jbossas/jboss-threads · GitHub which can reset the thread locals on these pooled threads. However, it isn't enabled/used in WildFly. We had a discussion about this for a similar issue, but I don't remember what conclusion we reached at on enabling this. Perhaps dmlloyd or jason.greene remember?

[emmartins]

You could use a ManagedExecutorService from the new EE Concurrency Utilities (JSR 236) to invoke the EJB instead of @Asynchronous. Since you are responsible for the task logic that invokes the EJB, you could cleanup the thread locals manually, if needed, when the task completes.