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

I think the main outstanding issues here are as follows.

First, are we convinced that it's necessary to add a try/finally block with a memory fence and a reachability fence in pretty much every mutator method? I think the answer is Yes, but we ought to have this ratified by memory model and GC experts.

A second issue is whether VarHandle::fullFence is the right memory fence operation. I'm not quite convinced that it is, but I'm not sure what the right alternative is either.

Finally, we probably also need approval from the maintainers of this code. :-)

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

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