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

Roger Riggs rriggs at openjdk.org
Fri Jul 22 14:48:04 UTC 2022


On Thu, 21 Jul 2022 21:35:02 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 two additional commits since the last revision:
> 
>  - simplify some comments
>  - Update ForceGC usage

LGTM 
(Except for a TODO and some tabs that should be spaces)
(Probably there's a higher level lesson to be learned about designing this kind of interaction with a server and the teardown needed).

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

> 515:             refEx = ne.refEx;
> 516:             listArg = ne.listArg;
> 517: 			// record a previous exception and quit if any limit is reached

Replace tabs with spaces.

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

PR: https://git.openjdk.org/jdk/pull/8311


More information about the core-libs-dev mailing list