2014/1/22 Mark Thomas <ma...@apache.org>: > On 16/01/2014 15:08, Mark Thomas wrote: >> On 15/01/2014 15:59, Rémy Maucherat wrote: >>> 2014/1/15 Mark Thomas <ma...@apache.org> >>> >>>> >>>> If folks integrating with Tomcat need to extend / replace what is >>>> currently in StandardContext.bindThread() and >>>> StandardContext.unbindThread() having a listener for this rather than >>>> having to extend StandardContext makes sense to me. >>>> >>> >>> Ok. >>> >>>> >>>> I'm not sure I follow what you are proposing for PAs. >>>> >>>> Nothing, if a utility class was doing the PA itself to make it easier and >>> do PA + setContextCL + call the listener, there would be more PAs (maybe a >>> performance impact). >> >> Thanks for the clarification. No objections here. > > 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. > 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. AFAIK, JNDI works in the following way: 1) org.apache.naming.java.javaURLContextFactory class is configured as the value of System.getProperty(Context.INITIAL_CONTEXT_FACTORY ). It is normally done by Catalina.initNaming() or by Tomcat.enableNaming() 2) javax.naming.spi.NamingManager.getInitialContext() calls javaURLContextFactory.getInitialContext() and it returns an instance of org.apache.naming.SelectorContext 3) SelectorContext uses either current thread or TCCL to find a JNDI context in a lookup table. Thus I think that the calls to ContextBindings.bindThread(...) / unbindThread(...) as performed by those StandardContext methods are needed only during some initial set up. Using them as general methods would be a waste. Those methods are overly synchronized and may be a nuisance. > - 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"? > - 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. > - 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. Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
