Summary: | The original class loader isn't re-bound to the thread in PersistentValve.invoke() | ||
---|---|---|---|
Product: | Tomcat 8 | Reporter: | Kyohei Nakamura <nakamura.kyohei.lab> |
Component: | Catalina | Assignee: | Tomcat Developers Mailing List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | minor | Keywords: | PatchAvailable |
Priority: | P2 | ||
Version: | 8.0.x-trunk | ||
Target Milestone: | ---- | ||
Hardware: | All | ||
OS: | All | ||
Attachments: |
patch against trunk
patch(rev.1) against trunk. |
Would it be possible to revert the changes (likely whitespace) that don't affect the code at all? It would make for a much cleaner and readable diff. Created attachment 32776 [details]
patch(rev.1) against trunk.
Thank you for your comment.
I attached the patch that ignored whitespace changes.
Great! That's much easier to see what you actually changed (just added a try/finally block)! It looked like you had made many more changes than that. Actually, the patch looks wrong. If the valve is set at the host level (or below), then nothing is needed, the host valve is the one supposed to be doing the context classloader setting. If somehow the valve is located at the engine level, then this looks rather weird (but maybe it's nice if there are lots of vhosts), and in that case it needs to set the context classloader (and some more - normally any such change is supposed to be done with context.(un)bind now). Since PersistentValve is not documented anywhere, the its use is questionable (non sticky sessions are not really allowed), I'd add a note in the javadoc saying it should be associated to a Host or Context. At the Host level the context class loaders are set in the StandardHostValve which is the basic Valve - i.e. always the last one in the chain. Therefore the clsss loader switching will be required for Hosts and Engines but not Contexts (or Wrappers although I can't think why this would ever be configued on a Wrapper). Very good point about the host, thanks. Fixed in trunk (for 9.0.x), 8.0.x (for 8.0.24 onwards) and 7.0.x (for 7.0.63 onwards). |
Created attachment 32769 [details] patch against trunk In PersistentValve.invoke(Request, Response) method, The Webapp class loader has been bound to the current thread. However, the original class loader that had been used before bind isn't restored to the thread. The original class loader should be re-bound to the thread.