RFR: 8362289: [macOS] Remove finalize method in JRSUIControls.java [v4]
Brent Christian
bchristi at openjdk.org
Thu Jul 17 23:56:47 UTC 2025
On Wed, 16 Jul 2025 22:45:21 GMT, Phil Race <prr at openjdk.org> wrote:
>> Eliminate a finalize() method in the Swing macOS implementation.
>>
>> I tested that the dispose method is being called by running this small test in combination with some 'println' statements in the source code (now removed)
>>
>> import javax.swing.JTabbedPane;
>> import javax.swing.plaf.TabbedPaneUI;
>>
>> public class CreateTabbedPaneUIStressTest {
>>
>> public static void main(String[] args) {
>> for (int i=0; i<1000000000; i++) {
>> JTabbedPane jtp = new JTabbedPane();
>> TabbedPaneUI tpui = jtp.getUI();
>> }
>> }
>> }
>>
>>
>> I also monitored the process size using 'top'.
>> And when I commented out the native call that did the free and re-tested I saw process size grow.
>>
>> Turning the above into a useful regression test doesn't seem possible to me.
>> Limiting Java heap is pointless (and I did use -Xmx20m) since it is a native resource that is to be freed and failure to dispose won't show up as a problem without taking down the machine.
>
> Phil Race has updated the pull request incrementally with one additional commit since the last revision:
>
> JRSUIControl.java
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()`
-------------
Changes requested by bchristi (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/26331#pullrequestreview-3031319703
More information about the client-libs-dev
mailing list