On Sat, 6 May 2023 12:47:21 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> src/jdk.naming.dns/share/classes/com/sun/jndi/dns/DnsClient.java line 165:
>>
>>> 163: // when this DNS client becomes phantom reachable
>>> 164: Selector sel = udpChannelSelector;
>>> 165: CleanerFactory.cleaner().register(this, () -> {
>>
>> I believe this is not quite right. The Cleanable returned by the `register`
>> method should be saved in a final field. And close() below should call
>> `Cleanable::clean` instead of `udpChannelSelector.close()`
>
>> I believe this is not quite right. The Cleanable returned by the `register`
>> method should be saved in a final field. And close() below should call
>> `Cleanable::clean` instead of `udpChannelSelector.close()`
>
> Selector::close is idempotent so it doesn't really matter but your approach
> would mean the cleanable is deregistered when the API is used correctly,
> which is good thing.
Thanks for the comments, implemented suggestions in
3a139b272b25d52b8be9593cee3aec3f373c12ff - new final field is added to save
`Cleaner.Cleanable` instance for later use in `DnsClient::close` to close the
selector and deregister the cleanable action.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13837#discussion_r1187563091
