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

[Alexey Ivanov]

On Mon, 9 Sep 2024 06:27:26 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> Several tests modified by https://github.com/openjdk/jdk/pull/19339 have been tweaked, see inline comments.
>> 
>> Notes:
>>  * We have a few XXXRepaint.java tests and in this patch, I updated all of them to follow the change added to the ListRepaint.java
>> 
>> @azvegint @aivanov-jdk please take a look.
>
> test/jdk/java/awt/Frame/MiscUndecorated/ActiveAWTWindowTest.java line 42:
> 
>> 40:     private volatile Frame frame, frame2;
>> 41:     private volatile Button button, button2;
>> 42:     private volatile TextField textField, textField2;
> 
> probably unnecessary but some of these fields might be used on different threads, so just in case.

Likely, it's unnecessary. I don't mind though.

I'd rather refactor the test to simplify it. Using `CountDownLatch` or other synchronisation primitives instead of wait/notify. I submitted [JDK-8339791](https://bugs.openjdk.org/browse/JDK-8339791). Feel free to re-assign if you like.

> test/jdk/java/awt/List/KeyEventsTest/KeyEventsTest.java line 68:
> 
>> 66:         KeyEventsTest app = new KeyEventsTest();
>> 67:         try {
>> 68:             app.initAndShowGui();
> 
> The test was changed [here](https://github.com/openjdk/jdk/pull/19339/files#diff-054074499ab6611e764a315fb13c51af4ad063b07f481e775a2262196077d285R69), the root cause of the failure discussed in https://github.com/openjdk/jdk/pull/19339/files#r1609189736 is a product bug: https://bugs.openjdk.org/browse/JDK-8201307.

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.

> test/jdk/java/awt/Paint/ListRepaint.java line 43:
> 
>> 41:         for (int i = 0; i < 10; ++i) {
>> 42:             try {
>> 43:                 EventQueue.invokeLater(ListRepaint::createAndShowGUI);
> 
> The purpose of the test is to run some methods of the AWT component on the main thread and check if it will be refreshed on the EDT. It was changed [here](https://github.com/openjdk/jdk/pull/19339/files#r1609190355). It is a product bug: https://bugs.openjdk.org/browse/JDK-8201307.

Yep, the previous update has changed the logic of the test so that it no longer reproduces the original problem for which it was written, namely [JDK-7090424](https://bugs.openjdk.org/browse/JDK-7090424).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20861#discussion_r1750768459
PR Review Comment: https://git.openjdk.org/jdk/pull/20861#discussion_r1750821284
PR Review Comment: https://git.openjdk.org/jdk/pull/20861#discussion_r1750839501