On Tue, 15 Jul 2025 19:57:56 GMT, Phil Race <p...@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.

src/java.desktop/macosx/classes/apple/laf/JRSUIControl.java line 115:

> 113:         flipped = other.flipped;
> 114:         cfDictionaryPtr = getCFDictionary(flipped);
> 115:         if (cfDictionaryPtr == 0) throw new RuntimeException("Unable to 
> create native representation");

Just a side topic to discuss: In the past I tried to look into this pattern and 
figure out is it possible for the GC to start cleaning the object after we 
allocate the native resource but before we register it with Disposer.addRecord. 
In such a case, the cleanup might never be executed.

Perhaps we should consider adding a reachabilityFence at the end of 
Disposer.addRecord, for both the DisposerRecord and the referent, to ensure 
they remain reachable. Similarly how it was done for cleaner:
https://hg.openjdk.org/jdk9/jdk9/jdk/rev/1f33e517236e
Or may be we can just use cleaner.

src/java.desktop/macosx/classes/apple/laf/JRSUIControl.java line 138:

> 136:             }
> 137:             cfDictionaryPtr = 0;
> 138:         }

Is it possible for this method to be called twice? It seems that not all other 
implementations include the ptr != 0 guard.

Also, synchronization appears to be used on the wrong object. I believe it 
should be synchronized on the outer JRSUIControl class, which uses 
synchronization to access the cfDictionaryPtr field. Alternatively, we might 
consider implementing synchronization in a different way.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26331#discussion_r2208586134
PR Review Comment: https://git.openjdk.org/jdk/pull/26331#discussion_r2208586380

Reply via email to