RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v12]
Roger Riggs
rriggs at openjdk.java.net
Tue Jun 7 18:48:25 UTC 2022
On Mon, 6 Jun 2022 21:59:56 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.
>
> Brent Christian has updated the pull request incrementally with one additional commit since the last revision:
>
> Restore EnumCtx to being an inner class, to keep all the state together in the code
trivial comments.
The commented out printf/println's should be removed before committing.
src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java line 152:
> 150: // Ensures that context won't get closed from underneath us
> 151: this.enumCtx.homeCtx.incEnumCount();
> 152: this.cleanable = LDAP_CLEANER.register(this, enumCtx);
Use `enumCtx.getHomeCts()` above.
Use `this.enumCtx` in call to `register`.
For consistency.
-------------
Marked as reviewed by rriggs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8311
More information about the core-libs-dev
mailing list