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

[Stuart Marks]

On Fri, 22 Jul 2022 20:51:59 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:
> 
>   remove some more tabs

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

> 156:             // reachable.
> 157:             // TODO: Is anything else needed so that this constructor
> 158:             //  "happens-before" the cleaning action ?

I think the resolution of this TODO is to modify the specification of Cleaner to require that:

1. the object being registered is guaranteed reachable until after registration has completed; and
2. a "memory consistency effects" clause should also be added to Cleaner.register. For an example, see the class specification of CopyOnWriteArrayList.

It may be that reachabilityFences are already in the right place in Cleaner. Great. But last time I looked it doesn't have any memory visibility operations. I'd suggest opening a bug on Cleaner for the spec change and accumulating notes there.

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

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