RFR: 8339561: The test/jdk/java/awt/Paint/ListRepaint.java may fail after JDK-8327401

Sergey Bylokhov serb at openjdk.org
Mon Sep 9 22:03:06 UTC 2024


On Mon, 9 Sep 2024 20:34:30 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> Not sure if it's a product bug…
>> 
>> AWT components do not explicitly state whether they're thread-safe or not. It's more or less assumed they should be. At the same time, I don't see code which ensures all the cases are thread-safe.
>> 
>> As far as I know, clientlibs group has been treating AWT components just like Swing ones: for safety create, access and destroy AWT components on EDT only.
>> 
>> I haven't looked at the test code thoroughly. It looks it still needs more investigation.
>> 
>> At the same time, I find it weird that `repaint` paints directly in its code and then posts paint event.
>> 
>> https://github.com/openjdk/jdk/blob/77468c284c068f921da543edd28333911e915b61/src/java.desktop/unix/classes/sun/awt/X11/XListPeer.java#L389-L390
>> 
>> Overall, it needs more investigation. It could be that the fix for [JDK-6471693](https://bugs.openjdk.org/browse/JDK-6471693) needs revising.
>
>>At the same time, I find it weird that repaint paints directly in its code and then posts paint event.
> 
> This is one of the implementation details of the AWT, the "native" component should be painted before the paint event will proceed on EDT, and even if EDT is blocked the "native" part should be painted. Since the XAWT is implemented via Swing(or some custom "java peers") which are not thread-safe we should use some additional synchronization. On macOS we have a special lock for that https://github.com/openjdk/jdk/blob/559fc711e03cf0086bea399ffb40cf294cbbb6e1/src/java.desktop/macosx/classes/sun/lwawt/LWListPeer.java#L87 and https://github.com/openjdk/jdk/blob/559fc711e03cf0086bea399ffb40cf294cbbb6e1/src/java.desktop/macosx/classes/sun/lwawt/LWComponentPeer.java#L361 but on XAWT it is quite "cumbersome" since it was implemented a long time ago.

And the NPE is triggered for List.select only because it is one method in the test that uses peer w/o synchronization.
https://github.com/openjdk/jdk/blob/77468c284c068f921da543edd28333911e915b61/src/java.desktop/share/classes/java/awt/List.java#L588

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20861#discussion_r1750991107


More information about the client-libs-dev mailing list