RFR: 8308038: java/util/concurrent/ThreadPerTaskExecutor/ThreadPerTaskExecutorTest.java timed out

[Daniel Fuchs]

On Sun, 21 May 2023 13:52:06 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> This is a test only change to the unit test for the ExecutorService returned by Executors.newThreadPerTaskExecutor. The tests for interrupting invokeAll assume the threads started to execute the tasks do actually execute the task code. The refresh in JEP 444 changed the implementation to use FutureTask, and FutureTask checks the interrupt status before it executes the task code. So some intermittent timeouts of the tests for interrupting invokeAll as those tests were waiting for the task to complete.
> 
> The main change is that the tests for interrupting invokeAll are changed to interrupt when the main thread blocks in invokeAll. They are also changed to check if the task started or not. The tests for interrupting invokeAny already did this, but these are changed to use the same infrastructure to avoid having two styles of tests in the same source file.

Looks reasonable. Just some question about testing invokeAll/invokeAny when there is only one task.

test/jdk/java/util/concurrent/ThreadPerTaskExecutor/ThreadPerTaskExecutorTest.java line 578:

> 576:             try {
> 577:                 scheduleInterruptAt("java.util.concurrent.ThreadPerTaskExecutor.invokeAny");
> 578:                 executor.invokeAny(Set.of(task));

Seems strange to test invokeAny when there is only one task?

test/jdk/java/util/concurrent/ThreadPerTaskExecutor/ThreadPerTaskExecutorTest.java line 835:

> 833:             try {
> 834:                 scheduleInterruptAt("java.util.concurrent.ThreadPerTaskExecutor.invokeAll");
> 835:                 executor.invokeAll(Set.of(task));

same remark for invokeAll?

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

PR Review: https://git.openjdk.org/jdk/pull/14072#pullrequestreview-1436244152
PR Review Comment: https://git.openjdk.org/jdk/pull/14072#discussion_r1200271736
PR Review Comment: https://git.openjdk.org/jdk/pull/14072#discussion_r1200272685