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 132: > 130: this.state = new CleaningAction(homeCtx, answer, > homeCtx.clnt); > 131: // Ensures that context won't get closed from underneath us > 132: this.state.homeCtx.incEnumCount(); Readability might be improved by using the new `homeCtx()` method instead of `state.homeCtx` in this file. It would then be consistent with how subclasses access it. It depends which style you prefer to be more consistent with. ------------- PR: https://git.openjdk.java.net/jdk/pull/8311
