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

Hi Peter, thanks for contributing to this. I think you're observing that as a practical matter, the implementations of GC and of various libraries such as reference queues must see a consistent view of memory in order for anything to work. This seems right. However, I'm concerned about what the _specification_ says, or ought to say.

Hans seems to think that the JLS assertions about finalization also apply to reference processing. I'm not so sure... that seems to me to be a rather generous interpretation. On the face of it, I cannot find anything explicit in the specifications that supports it. It certainly seems reasonable (to me) that reachabilityFence(x) ought to HB the corresponding reference being dequeued. If so I would like the specification to say that. (The fact that there is a GC thread that enqueues the reference is part of the implementation, which is opaque to the specification.) It may also be the that any point where the referent is reachable HB its corresponding reference is dequeued. Clearly the GC implementation needs to make sure this is the case; the questions in my mind are whether, where, and how this should be specified!

It may be that the VarHandle fences aren't necessary. However, if they end up driving the right updates to the specifications, they will have served their purpose.

Setting this aside, it does seem like all uses of a cleanable object need to have a try/finally statement, with at least an RF in the finally clause. Is there any evidence that shows that this construct isn't needed?

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

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