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

[Daniel Fuchs]

On Thu, 15 May 2025 23:59:41 GMT, Brent Christian <bchristi at openjdk.org> wrote:

>> Please review this change to replace the finalizer in `AbstractLdapNamingEnumeration` with Cleaner.
>> 
>> (The [first PR](https://github.com/openjdk/jdk/pull/8311) for this fix started some substantial discussions, leading to, among other things, the [8314480](https://bugs.openjdk.org/browse/JDK-8314480) `java.lang.ref` memory ordering spec update.)
>> 
>> In standard fashion, pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult res`, and `LdapClient enumClnt`) are moved into a _Context_ object. 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). Use of `reachabilityFence()` ensures memory visibility on the Cleaner thread (per the aforementioned spec update).
>> 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`. It confirms that the use of Cleaner does not keep an `LdapSearchEnumeration` object reachable. The other `AbstractLdapNamingEnumeration` subclasses (`LdapNamingEnumeration` and `LdapBindingEnumeration`) can be expected to behave the same.
>> 
>> Thanks.
>> 
>> **Edit: (Re)viewers: due to there being a lot of indentation changes, you might consider enabling the "Hide whitespace" option on the "Files changed" tab. To my eye, it gives a better view of the changes.**
>
> Brent Christian has updated the pull request incrementally with one additional commit since the last revision:
> 
>   update copyright

Generally looks good. I haven't verified whether all methods that need a reachabilityFence have it. This will require further analysis (from me).
As Alan noted it would be really good if we could use try-with-resource, but I am not expert enough in LDAP search implementation to figure out whether that's feasible.

Your proposed fix preseves the status quo - where a finalizer has been replaced by a cleaner - and asserting that the two are equivalent seems more trackable for now.

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

> 451:         try {
> 452:             // Cleanup previous context first
> 453:             getHomeCtx().decEnumCount();

What is the reason for calling `getHomeCtx()` here when we use `enumCtxt.homeCtx` a few lines below?

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

PR Review: https://git.openjdk.org/jdk/pull/25242#pullrequestreview-2914626239
PR Review Comment: https://git.openjdk.org/jdk/pull/25242#discussion_r2138506580