On Mon, 29 Jan 2024 19:00:46 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Nir Lisker has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update tests
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1352#discussion_r1470247519
