Check parent done condition before rejecting extraneous worker

[mpkorstanje]

The WorkerThreadPoolHierarchicalTestExecutorServiceTests.limitsWorkerThreadsToMaxPoolSize is flaky with a rather puzzling exception. While there are 3 permits available, 1 worker active thread. And yet the task rejected from the pool.

Caused by: java.util.concurrent.RejectedExecutionException: Task with WorkerLeaseManager [parallelism = 3, semaphore = java.util.concurrent.Semaphore@5af97070[Permits = 3]] rejected from java.util.concurrent.ThreadPoolExecutor@f5237bd2[Running, pool size = 3, active threads = 1, queued tasks = 0, completed tasks = 2]
	at org.junit.platform.engine.support.hierarchical.WorkerThreadPoolHierarchicalTestExecutorService$LeaseAwareRejectedExecutionHandler.rejectedExecution(WorkerThreadPoolHierarchicalTestExecutorService.java:922)

This happens because, given the following test tree with a max of 3 workers, worker 1 starts execution at the root node:

root  <- worker 1
 |- child1
 | |- leaf1a
 | |- leaf1b
 |- child2
 | |- leaf2a
 | |- leaf2b

When the child nodes are executed, there are 2 worker threads active and 1 available. The state at this point looks like this:

root
 |- child1 <- worker 1
 | |- leaf1a 
 | |- leaf1b 
 |- child2  <- worker 2
 | |- leaf2a
 | |- leaf2b

Both workers will then race each other trying to start 1 more. The winner of that race, child1 for the sake of argument, will then execute the leaf1a, while the other thread will start working on the second leaf1b node.

The state at this point looks like this:

root
 |- child1
 | |- leaf1a <- worker 1
 | |- leaf1b <- worker 3
 |- child2
 | |- leaf2a <- worker 2
 | |- leaf2b

Then a second race condition happens. After leaf1a is done, the thread starts to wait for leaf1b to be finished. While it is waiting, it returns its worker lease and tries to compensate. At the time is tries to compensate, both worker 2 and worker 3 are still busy and the executor rejects the task.

The state at this point looks like this:

root
 |- child1  <- worker 1 (awaiting leaf1b)
 | |- leaf1a
 | |- leaf1b <- worker 3
 |- child2
 | |- leaf2a <- worker 2
 | |- leaf2b

Then a second race condition happens. After leaf1a is done, the thread starts to wait for leaf1b to be finished. While it is waiting, it returns its worker lease and tries to compensate. At the time is tries to compensate, both worker 2 and worker 3 are still busy and the executor rejects the task.

By the time the RejectedExecutionHandler is invoked, worker 2 and 3 have finished their work and returned their leases. The state at this point looks like this:

root
 |- child1 <- worker 1 (awaiting leaf1b)
 | |- leaf1a
 | |- leaf1b 
 |- child2
 | |- leaf2a 
 | |- leaf2b

At this point the executor.isShutdown() || workerLeaseManager.isAtLeastOneLeaseTaken() condition evaluates to false because 1) the executor is not shutdown and 2) there are no workers with active leases so an exception is thrown. By also checking if the doneCondition of the worker that tried to start the new worker we can see that it was waiting in vain.

---

I hereby agree to the terms of the JUnit Contributor License Agreement.

---

Definition of Done

• There are no TODOs left in the code
• Method preconditions are checked and documented in the method's Javadoc
• Coding conventions (e.g. for logging) have been followed
• Change is covered by automated tests including corner cases, errors, and exception handling
• Public API has Javadoc and @API annotations
• Change is documented in the User Guide and Release Notes

[testlens-app]

🔎 No tests executed 🔎

🏷️ Commit: 6966ccb
▶️ Tests: 0 executed
🟡 Checks: 0/12 completed

---

Learn more about TestLens at testlens.app.

[marcphilipp]

Thanks for the detailed analysis! 👍