Mark, On 1/22/14, 5:21 AM, Mark Thomas wrote: > On 22/01/2014 09:42, Konstantin Kolinko wrote: >> 2014/1/22 Mark Thomas <ma...@apache.org>: > > <snip/> > >>> I've been looking at this some more with an eye to reducing code >>> duplication. I think the thread binding listener could be merged with >>> the ContainerListener. I can't see a good reason to keep them apart at >>> this point. >> >> 1. If there are many ContainerListeners installed you will be calling >> them with an event that they are not interested to handle. It wastes >> time. > > Fair point. This is something that gets called several times on every > request. That probably makes the case for a separate listener. > >> >>> With this in mind, the changes I'm planning are: >>> >>> - Add bindThread() and unbindThread() to the Context interface >> >> Do you mean as a generic replacement to Thread.setContextClassLoader() calls? >> I think the current methods of StandardContext are not suitable for >> such exposure. > > Sort of, with some refactoring because of the JNDI aspects you > mentioned. My main aim is to reduce code duplication. > >>> - Add optional PA (as in callers can opt to use it if they wish) >>> support to bindThread() and unbindThread() >> >> What do you mean by "PA"? > > PrivilegedAction > >>> - Switch all the components currently implementing their own >>> bind/unbind methods to use these new context methods >>> - Switch container event type to use an enum >> >> Enums are better than Strings for performance, >> but I am not sure whether those can be extended if needed. >> >> I'd prefer to keep Strings here. > > Do we need to extend these? This is all internal to Tomcat. We can add > values to the enum as easily as we can use new String values. > >>> - Add bind and unbind events to this new enum >>> - Remove ThreadBindingListener >>> >>> The main benefit of this is that all the actions required on bind and >>> unbind will be in a single place rather than duplicated (sometimes >>> inconsistently) across the code base. > > This is really what I am aiming at. There are several places across the > code that do call near enough exactly the code to bind and unbind the > context class loader. It is this duplication I want to remove. > > I'll start again and address my primary concern - the duplicate code. > The other aspects I'm happy to think about / discuss further with a view > to possibly coming back to them later.
Could this be done more simply by providing a pair of utility methods to do the enter/exit work rather than treating them as events to be "handled"? That way, they can be invoked directly by components that need them and ignored by those that don't. -chris
signature.asc
Description: OpenPGP digital signature
