search

pukkaone / webappenhance

Java web application enhancements library. Compile JSPs on startup. Escape JSP EL values to prevent cross-site scripting.
34 stars 10 forks source link

ThreadLocal leak on EscapeXmlELResolver #1

Closed rvt closed 11 years ago

rvt commented 11 years ago

disclaimer : I am not very knowledgeable on the ThreadLocal subject!

In the class EscapeXmlELResolver there is a ThreadLocal used that is not cleaned up during re-deployments. For example Tomcat will throw this during un-deployments:

Apr 24, 2013 12:03:13 PM org.apache.catalina.loader.WebappClassLoader checkThreadLocalMapForLeaks SEVERE: The web application [] created a ThreadLocal with key of type [com.github.pukkaone.jsp.EscapeXmlELResolver$1](value [com.github.pukkaone.jsp.EscapeXmlELResolver$1@5120fc3b]) and a value of type [java.lang.Boolean](value [false]) but failed to remove it when the web application was stopped. This is very likely to create a memory leak.

Looking at the code, and understand how Context and a ELResolver's work, wouldn't it not be enough to have a local variable (private boolean excludeMe=false;) and use that?

As far as I know is that a Page contexts may be pooled internally, but not shared by multiple JSP pages at the same time, there for I don't think it's needed to use a ThreadLocal within EscapeXmlELResolver at all?

Kind Regards, R. van Twisk

pukkaone commented 11 years ago

A ThreadLocal instead of a simple field (private boolean excludeMe = false;) is necessary in case multiple threads execute a single instance of EscapeXmlELResolver.