On Fri, 27 May 2022 22:00:24 GMT, Brent Christian <bchri...@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: > > Threading-related fixes `AbstractLdapEnumeration`'s mutable state brings the possibility of threading issues between the program and cleaner threads. I've added some threading-related changes to the fix. See my [comment in the bug report](https://bugs.openjdk.java.net/browse/JDK-8283660?focusedCommentId=14498566&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14498566) for additional background. Since synchronization may now happen on the cleaner thread, I've changed `AbstractLdapEnumeration` to use its own `Cleaner` instance instead of the shared cleaner, for added safety. There have been deadlocks in ldap cleanup in the past. The added `finally` blocks led to a lot of indentation changes. The "hide whitespace" option might help with viewing the changes. ------------- PR: https://git.openjdk.java.net/jdk/pull/8311
