Hi,
I just committed a fix to the trunk based on your patch, thanks.
I verified using visual VM on the Tomcat process that the Errors class
gets garbed collected on undeployment of the application.
Paul.
On Oct 25, 2010, at 1:56 PM, Paul Sandoz wrote:
> Hi Philippe,
>
> Thanks for the patch,
>
> On Oct 22, 2010, at 10:33 AM, Philippe Marschall wrote:
>
>> Hi
>>
>> I have a ClassLoader leak that looks like it's caused by
>> com.sun.jersey.spi.inject.Errors. My application is deployed in
>> Tomcat
>> and Jersey 1.4 is deployed with the application in WEB-INF/lib. [1]
>> Shows the shortest path to a GC root of a class after undeployment
>> (done
>> with Eclipse MAT). According to it there's an instance of Errors
>> still
>> in a ThreadLocal preventing it, it's class and the WebappClassLoader
>> from becoming eligible for garbage collection.
>>
>> The first thing I tried was replacing the anonymous TheadLocal in
>> Errors
>> with a normal TheadLocal
>
>> (why is #initalValue() synchronized BTW?) and
>
> It is a silly redundancy, because i dumbly copied the pattern of the
> example in the ThreadLocal JavaDoc of SE 5.
>
> http://download.oracle.com/javase/1.5.0/docs/api/java/lang/ThreadLocal.html
>
>
>> remove()ing it even if there are no messages (see patch). That
>> however
>> did not solve the problem.
>>
>> In the end I wrote a ServletRequestListener that .remove()s
>> Errors#errors after each request. That fixed the problem. This
>> leads me
>> to believe that somehow Errors#errors is not cleaned up after each
>> request.
>>
>
> It should be cleaned up after servlet initialization. There should
> be no need to do it for every request,
>
>
>> Since my application is fairly simple I was wondering whether other
>> people how deploy Jersey in Tomcat or Jetty have seen similar issues?
>>
>
> I have not, but from your analysis i suspect it is happening for the
> case when no errors are detected. FWIW GlassFish, on undeployment,
> will go through all classes loaded by the Web app classloader and
> set static fields to null, to avoid this type of leak.
>
> I am not sure why your patch to the Errors.postProcess method does
> not work. Perhaps because there were other deployments? Have you
> tried from a fresh re-start of Tomcat?
>
> Any access to error state should only be allowed within the call/
> scope of the Errors.processWithErrors method. I have strengthened
> this constraint, see modified code at the end of the email.
>
> Investigating...
>
> Paul.
>
> private static ThreadLocal<Errors> errors = new
> ThreadLocal<Errors>();
>
> public static interface Closure<T> {
> public T f();
> }
>
> public static <T> T processWithErrors(Closure<T> c) {
> Errors e = errors.get();
> if (e == null) {
> e = new Errors();
> errors.set(e);
> }
> e.preProcess();
>
> RuntimeException caught = null;
> try {
> return c.f();
> } catch (RuntimeException re) {
> // If a runtime exception is caught then report errors and
> // rethrow
> caught = re;
> } finally {
> e.postProcess(caught == null);
> }
>
> throw caught;
> }
>
> private static Errors getInstance() {
> Errors e = errors.get();
> // No error processing in scope
> if (e == null) {
> throw new IllegalStateException("There is no error
> processing in scope");
> }
> // The following should not be necessary but given the
> fragile nature of
> // static thread local probably best to add it in case some
> internals of
> // this class change
> if (e.stack == 0) {
> errors.remove();
> throw new IllegalStateException("There is no error
> processing in scope");
> }
> return e;
> }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe_at_jersey.dev.java.net
> For additional commands, e-mail: users-help_at_jersey.dev.java.net
>