On Wed, 16 Jul 2025 22:45:21 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.
>
> Phil Race has updated the pull request incrementally with one additional
> commit since the last revision:
>
> JRSUIControl.java
Marked as reviewed by serb (Reviewer).
>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.
-------------
PR Review: https://git.openjdk.org/jdk/pull/26331#pullrequestreview-3031678484
PR Comment: https://git.openjdk.org/jdk/pull/26331#issuecomment-3086587833
