On Wed, 20 Apr 2022 00:32:25 GMT, Brent Christian <bchri...@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