Bug 50556 - improve JreMemoryLeakPreventionListener against leak caused by LdapPoolManager
Summary: improve JreMemoryLeakPreventionListener against leak caused by LdapPoolManager
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 6.0.29
Hardware: All All
: P2 enhancement (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-06 17:13 UTC by Sylvain Laurent
Modified: 2011-01-08 20:01 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sylvain Laurent 2011-01-06 17:13:06 UTC
When class com.sun.jndi.ldap.LdapPoolManager is initialized, if if the system property com.sun.jndi.ldap.connect.pool.timeout is set to a value greater than 0, a PoolCleaner thread is spawned, without fixing a specific context class loader.
If the initialization of the class is triggered by a web application, its class loader will be used by the PoolCleaner thread. If that web app is stopped, its class loader will leak.

We can improve JreMemoryLeakPreventionListener to prevent that leak.
Comment 1 Christopher Schultz 2011-01-06 17:41:57 UTC
What are our opportunities to initialize this class before webapp code gets involved? Is this something that is safe to initialize during Tomcat startup, or will cause negative effects for the webapp(s) using LDAP services?
Comment 2 Sylvain Laurent 2011-01-06 17:48:01 UTC
the JreMemoryLeakPreventionListener is there to initialize such leaking classes.
And yes, it is safe to initialize com.sun.jndi.ldap.LdapPoolManager during tomcat startup.
Comment 3 Christopher Schultz 2011-01-06 17:55:20 UTC
(In reply to comment #2)
> the JreMemoryLeakPreventionListener is there to initialize such leaking
> classes.

Yup, I've worked on it.

> And yes, it is safe to initialize com.sun.jndi.ldap.LdapPoolManager during
> tomcat startup.

Excellent: just initialize the class inside of the CTTL push/pop that's already in there and you should be okay. I guess you'd only have to do it if com.sun.jndi.ldap.connect.pool.timeout > 0, though system properties can be changed at runtime, so it might be a good idea to temporarily set it to 1 if it's currently 0, init the class, then set it back.
Comment 4 Sylvain Laurent 2011-01-07 18:01:14 UTC
>I guess you'd only have to do it if com.sun.jndi.ldap.connect.pool.timeout > 0, though system properties can be changed at runtime, so it might be a good idea to temporarily set it to 1 if it's currently 0, init the class, then set it back.

I don't think this is a good idea. This would force the leaking thread to be spawned even in cases where it is never spawned (if com.sun.jndi.ldap.connect.pool.timeout=0). This property is supposed to be set on the command line, so that it does not change at runtime.

I committed the fix on trunk, will be ready for 7.0.6
Comment 5 Sylvain Laurent 2011-01-07 18:19:57 UTC
proposed backport on Tomcat 6, moving this BZ issue to tomcat 6.
Comment 6 Konstantin Kolinko 2011-01-08 20:01:29 UTC
Implemented in 6.0 with r1056851 and will be in 6.0.30.