[jira] Commented: (WW-1544) Dispatcher threadlocal cleanup does not clean up the threadlocal

["Don Brown (JIRA)" <ji...@apache.org>]

    [ http://issues.apache.org/struts/browse/WW-1544?page=comments#action_39030 ] 
            
Don Brown commented on WW-1544:
-------------------------------

The purpose of the Dispatcher class is to try to bind the framework and its objects to filter and portlet class instances rather than globally.  The goal would be to be able to choose different object factories, different factory implements, and different configurations for multiple instances of a filter and/or portlet dispatcher.

Unfortunately, chunks of Struts 2 still use statics and therefore, are unable to be cleanly separated.  How we deal with these global statics when multiple cleanup/destroy events can be called is a good question.  In the mentioned ObjectFactory case, we should be able to tie the ObjectFactory instance to a dispatcher so that it's destory method would in fact be called only once.  DispatcherListener instances are passed the Dispatcher instance, so there should still be one notification per dispatcher instance.

So, to answer your question, there should be one cleanup per dispatcher, and one dispatcher per filter or portlet.

> Dispatcher threadlocal cleanup does not clean up the threadlocal
> ----------------------------------------------------------------
>
>                 Key: WW-1544
>                 URL: http://issues.apache.org/struts/browse/WW-1544
>             Project: Struts 2
>          Issue Type: Bug
>    Affects Versions: 2.0.1
>            Reporter: Rickard Öberg
>
> In Dispatcher.cleanup() there is supposedly code to do (as the JavaDoc states) "Cleans up thread local variables". 
> However, the actual codes does "instance.set(null);" which is not a threadlocal cleanup. It only sets the reference of the current thread to null, meaning, there may be a bunch of other threads with references to Dispatchers. 
> The proper way to clean up a threadlocal is to set it to null, i.e. "instance = null".
> However, it appears that this breaks pretty much all tests. The basic usage of how Dispatchers are used is, it seems, messed up. If it is supposed to be some local context only, then it should be set and cleared immediately, e.g. in FilterDispatcher.doFilter(), instead of leaving Dispatcher references hanging possibly indefinitely.
> Apart from fixing this I would suggest that the way these instances are managed should be rethought. If, for example, both FilterDispatcher and Jsr168Dispatcher are used (as we do), then (if Jsr168Dispatcher also does Dispatcher.cleanup, as it should) there will be many calls to cleanup() and hence possibly many ObjectFactory.destroy() and dispatcherListener.dispatcherDestroyed() calls. It appears that Dispatcher is a bit schizophrenic with regard to who owns and manages it.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/struts/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira