On Thu, 4 May 2023 15:30:20 GMT, Andy Goryachev <[email protected]> wrote:

>> tests/system/src/test/java/test/javafx/embed/swing/SwingNodeMemoryLeakTest.java
>>  line 80:
>> 
>>> 78:         });
>>> 79: 
>>> 80:         SwingUtilities.invokeAndWait(() -> Util.sleep(500));
>> 
>> Sleeping on either the JavaFX Application thread or the Swing EDT (as in 
>> this case) is not generally desirable. What is the sleep trying to achieve? 
>> Is there a better way, possibly sleeping on the test thread and then doing a 
>> `SwingUtilities.invokeAndWait` (maybe with a no-op runnable? not sure)?.
>
> I can't believe I missed that!
> I wonder if we could use CountDownLatch instead here and use 
> Util.await(CountDownLatch) or something like that.

This is something that came from the original patch - I'm no Swing expert so I 
originally figured I should leave it here. My only guess was that it (among 
other `Thread.sleep()`-s in this patch) is trying to let the other threads run 
for a bit and chew through the resources to make sure they're collectable.

Upon closer inspection it looks like JMemoryBuddy is already doing those 
sleeps, so I'll remove them - tests should work fine without those.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1185739169

Reply via email to