RFR: 8362289: [macOS] Remove finalize method in JRSUIControls.java [v4]

[Phil Race]

On Fri, 18 Jul 2025 03:34:49 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

> > Synchronized methods might be OK without, but the following methods are not synchronized, and access cfDictionaryPtr:
> 
> Unfortunately this is actually true, in awt we even have a special class [CFRetainedResource](https://github.com/openjdk/jdk/blob/04c0b130f09c093797895cc928fe020d7e584cb9/src/java.desktop/macosx/classes/sun/lwawt/macosx/CFRetainedResource.java#L36) which maintains a locks around the native pointer and prevents its usage after de-allocation(intentional or via finalize). It was implemented as part of JDK-8164143.
> 
> I can migrate the current API of DIsposer to the something similar to CFRetainedResource as a follow up bug. The current one did not introduce the new issues, since implementation via finalize has the same bug.

What does that mean ?

CFRetainedResource is extended by a lot of classes and it is hard to figure out how correctly it is used.
I would take a big step back and look before extending what it does.
And it also uses finalize which is why I looked and found what a mess it is.

> I would recommend adding a `reachabiltyFence()` to `Disposer.addRecord()`, as @mrserb [suggests](https://github.com/openjdk/jdk/pull/26331/files#r2208586134).
> 
> Unexpected unreachability is a subtle problem with GC-assisted APIs - finalizers, `Cleaner`, and even direct use of `References` & `ReferenceQueues`. In particular, the VM can decide than object is unreachable even when a method on that object is still running.
> 
> For this reason, our standard recommendation is that any instance method that accesses the cleanable state (in this case, `cfDictionaryPtr`) should be enclosed with a `try{...}finally{ reachabilityFence(); }` block.
> 
> Synchronized methods _might_ be OK without, but the following methods are not synchronized, and access `cfDictionaryPtr`:
> 
> * `getHitForPoint()`
> * `getPartBounds()`
> * `getScrollBarOffsetChange()`
> * `sync()`

I do not see any scenario in which this is needed.
I will evaluate if we need to  file a bug to use reachabiltyFence in the Disposer creation since that is a one-time contained thing but  I have no reason to suppose it is needed in this class.

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

PR Comment: https://git.openjdk.org/jdk/pull/26331#issuecomment-3100996792
PR Comment: https://git.openjdk.org/jdk/pull/26331#issuecomment-3101010453