RFR: 8324658: Allow animation play/start/stop/pause methods to be called on any thread [v9]

[Kevin Rushforth]

On Mon, 29 Jan 2024 15:39:32 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> tests/system/src/test/java/test/com/sun/javafx/animation/SynchronisityTest.java line 65:
>> 
>>> 63:         }
>>> 64: 
>>> 65:         waiter.await(GRACE_PERIOD, TimeUnit.SECONDS);
>> 
>> I don't think this will do what you want. This will return almost immediately, before any of the animation has run (as soon as the runnable that sets the uncaught exception handler finished). What I think you need is something more like this:
>> 
>> 
>>         Platform.runLater(() -> registerExceptionHandler());
>>         assertTrue(waiter.await(GRACE_PERIOD, TimeUnit.SECONDS));
>> 
>>         // start the 10 threads...
>> 
>>         Util.sleep(GRACE_PERIOD);
>> 
>> 
>> The await ensures that the uncaught exception handler has been registered before starting the threads. The sleep then runs the threads for a fixed amount of time.
>
>> I don't think this will do what you want. This will return almost immediately, before any of the animation has run (as soon as the runnable that sets the uncaught exception handler finished).
> 
> I observed that this setup works correctly for the `AnimationTimer` test. The test does wait for `GRACE_PERIOD` seconds when there is no exception and then succeeds, or catches and fails immediately when there onw is thrown. The `Animation` test was more finicky, but I think that is because I'm not simulating a processing load properly.
> 
>> The await ensures that the uncaught exception handler has been registered before starting the threads. 
> 
> While technically true, the handler register in a negligible amount of time compared to the grace period. Because exceptions are thrown continuously )it's not a one-time event), the handler will never miss a failure.
> 
> I will change it for correctness sake.

I misread it. I see now that you only countdown when there is a failure. In that case, it seems fine as you have it (unless you also want to have a second latch that ensures that the uncaught event handler is in place prior to submitting the runnables). You might want to add a comment as to the purpose of the latch, since it wasn't initially obvious to me.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1352#discussion_r1469812959