RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner

Roger Riggs rriggs at openjdk.java.net
Wed Apr 20 18:57:45 UTC 2022


On Wed, 20 Apr 2022 00:32:25 GMT, Brent Christian <bchristi at openjdk.org> wrote:

> Please review this change to replace the finalizer in `AbstractLdapNamingEnumeration` with a Cleaner.
> 
> The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult res`, and `LdapClient enumClnt`) are moved to a static inner class . From there, the change is fairly mechanical.
> 
> Details of note: 
> 1. Some operations need to change the state values (the update() method is probably the most interesting).
> 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read `homeCtx` from the superclass's `state`.
> 
> The test case is based on a copy of `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test case might be possible, but this was done for expediency.
> 
> The test only confirms that the new Cleaner use does not keep the object reachable. It only tests `LdapSearchEnumeration` (not `LdapNamingEnumeration` or `LdapBindingEnumeration`, though all are subclasses of `AbstractLdapNamingEnumeration`). 
> 
> Thanks.

src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java line 59:

> 57:      * cleanup, which happens by calling cleanup(), or is done by the Cleaner.
> 58:      */
> 59:     private static class CleaningAction implements Runnable {

This nested class might be more aptly named  `EnumCtx` since it is used during the enumeration process.

The mention of the `cleanup()` method in the nested class javadoc is a bit ambiguous, it seems there should be a cleanup() method in the class, but it is in AbstractLdapNamingEnumeration.

The `state` field might also be renamed `enumCtx`.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8311


More information about the core-libs-dev mailing list