Skip to content

Make immediate-cancel task termination test deterministic#36967

Open
junhyeong9812 wants to merge 1 commit into
spring-projects:mainfrom
junhyeong9812:fix/simpleasync-flaky-immediate-cancel
Open

Make immediate-cancel task termination test deterministic#36967
junhyeong9812 wants to merge 1 commit into
spring-projects:mainfrom
junhyeong9812:fix/simpleasync-flaky-immediate-cancel

Conversation

@junhyeong9812

@junhyeong9812 junhyeong9812 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Overview

As discussed in #36965, this is a separate, test-only fix for the flaky SimpleAsyncTaskExecutorTests.taskTerminationTimeoutWithImmediateCancel test (the unrelated failure that turned CI red on that pull request). Production behavior is unchanged.

Problem

The test submits a task and immediately closes the executor, then asserts the future was cancelled. The cancellation flag is set by close() on the calling thread, while it is read at the start of the task in TaskTrackingRunnable.run() on a separate worker thread. There is no ordering guarantee between the two: if the worker thread reaches the cancellation check before close() sets the flag, it passes the check, the trivial task completes normally, and the future is never cancelled, so future.get() returns instead of throwing. Under CI load the worker is more likely to win that race, which is why it fails intermittently.

I reproduced this with a tight loop of the same scenario: roughly 2 to 4 failures per 50,000 iterations locally, which becomes more frequent on a loaded runner.

Fix

Override doExecute to capture the task-tracking wrapper instead of starting a background thread, then run the wrapper on the test thread after close() has set the cancellation flag. The same production cancellation path (checkCancelled to future.cancel(false) to CancellationException) is exercised, but the read and write now happen in program order on a single thread, so the race is gone. With the wrapper never started, activeThreads is empty and the termination wait is skipped, so the test is also faster. The same capture technique is already used by cancelledThrottledTaskReleasesPermit. The deterministic version passes 50,000 of 50,000 iterations.

The taskTerminationTimeoutWithImmediateCancel test submitted a task and
immediately closed the executor, then asserted that the future was
cancelled. The cancellation flag is set by close() on the calling thread,
while it is checked at the start of the task on a separate worker thread.
With no ordering guarantee between the two, a quickly scheduled worker
could pass the cancellation check before close() set the flag, complete
the trivial task normally, and leave the future uncancelled, making the
test fail intermittently under load.

Override doExecute to capture the task-tracking wrapper instead of
running it on a background thread, then run it on the test thread after
close() has set the cancellation flag. This exercises the same
cancellation path deterministically, with no reliance on thread
scheduling.

Signed-off-by: junhyeong9812 <pickjog@gmail.com>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: waiting-for-triage An issue we've not yet triaged or decided on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants