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

Reply via email to