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. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
