On Tue, 15 Jul 2025 20:24:46 GMT, Sergey Bylokhov <s...@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 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. I am not *expecting* it to be called twice. I'm just saying that with this check it would be safe if it were. Not sure I even need synchronization here. Only the Disposer will have a reference. So I could remove synchronized, remove the != 0 check ... but I don't see a problem either way. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26331#discussion_r2208841918