On Mon, 29 Jan 2024 21:35:24 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> tests/system/src/test/java/test/com/sun/javafx/animation/SynchronizationTest.java
>> line 87:
>>
>>> 85: for (int i = 0; i < 10; i++) {
>>> 86: executor.submit(runnable);
>>> 87: }
>>
>> I did a quick prototype of the idea I mentioned in my last comment, and it
>> looks like it works:
>>
>>
>> Runnable wrappedRunnable = () -> {
>> try {
>> runnable.run();
>> } catch (Throwable e) {
>> thread = Thread.currentThread();
>> throwable = e;
>> failed.set(true);
>> waiter.countDown();
>> }
>> };
>>
>> for (int i = 0; i < 10; i++) {
>> executor.submit(wrappedRunnable);
>> }
>>
>>
>> This obviates the need for the `registerExceptionHandler` calls in the two
>> tests themselves (this one in _this_ method is still needed).
>
> I don't think the executor service catches the exception because if it did
> then the `AnimationTimer` test would also always pass. I do get the AIOOB
> exceptions from the background thread caught in the handler (which fails the
> test). This test is rather stable for me. The `Animation` one is more
> problematic, sometimes failing and sometimes not when it should fail.
Based on the testing I did, the executor service does prevent at least some
exceptions that happen on the test thread from reaching the uncaught exception
handler. An explicit try/catch will catch those.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1352#discussion_r1470290986
