On Thu, 26 Feb 2026 11:48:11 GMT, Daniel Fuchs <[email protected]> wrote:
>> Can I please get a review of this change which addresses an issue in the >> `java.naming` module? >> >> As noted in https://bugs.openjdk.org/browse/JDK-8273874 when a >> `javax.naming.Context` is constructed backed by a >> `com.sun.jndi.ldap.LdapCtxFactory`, the internal implemenation of `LdapCtx` >> can lead to creation of Threads that are used for the managing connections >> and for managing event notifications. These threads are system threads. >> However, the way they are created currently, they end up capturing the >> context classloader of the calling Thread. This classloader will be held >> onto as long as these Threads stay alive and can thus prevent the >> classloader from being unreferenced. >> >> The change in this PR replaces the creation of these threads with the >> `InnocuousThread`s, which do not have a context classloader associated with >> them. >> >> A new jtreg test has been introduced to reproduce the issue and verify the >> fix. Existing tests continue to pass with this change. > > test/jdk/com/sun/jndi/ldap/LdapTCCLTest.java line 169: > >> 167: // add a NamingListener to exercise the Thread creation >> in the internals >> 168: // of LdapCtx >> 169: ctx.addNamingListener(LOOKUP_NAME, >> EventContext.OBJECT_SCOPE, new Listener()); > > I wonder, before your fix, when the Listener is invoked, is it invoked from a > thread whose TCCL is the `urlc`? If so, some listener implementation may be > depending on it, and may start failing with CNFE or CCE if the TCCL is no > longer set. > In other words - could this change be observed by custom code which might > depend on the old behaviour? Thank you for the review Daniel. I went through the code and from what I can see, in some cases, the listener implementation might have been called from the Thread whose context classloader would have been the one captured when that Thread was created. I've now created a CSR https://bugs.openjdk.org/browse/JDK-8380897 to track this behavioural change. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29934#discussion_r2988047690
