RFR: 8376308: java/net/httpclient/CancelRequestTest.java fails intermittently with "Expected CancellationException not received"

[Volkan Yazici]

On Mon, 26 Jan 2026 13:09:16 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

> When using async mode, and if the "wrong" test/client thread gets suspended at the wrong time there's a small time window in which the server might be able to send its reply before the request is cancelled.
> This can be avoided by having the server handler wait on a semaphore until the cancellation exception has been propagated to the caller on the client side.

test/jdk/java/net/httpclient/CancelRequestTest.java line 97:

> 95: 
> 96:     private static final Random random = RandomFactory.getRandom();
> 97:     private static final ConcurrentHashMap<String, Semaphore> semaphores

Our `Semaphore` usage has two stages:

1. wait — used by the server handler to acquire permit to write the response
2. advance — used by the test method to allow the server handler to advance

Can we simplify the design by replacing the usage of `Semaphore` with `CountDownLatch`? This will also avoid the need for a magical `RELEASE_ALL` constant.

test/jdk/java/net/httpclient/CancelRequestTest.java line 396:

> 394:                 throw new AssertionError("Expected CancellationException not received");
> 395:             } catch (ExecutionException x) {
> 396:                 sem.release(RELEASE_ALL);

Have you considered having a single `sem.release(RELEASE_ALL)` in a `finally` block?

Note that this comment applies to more places in the change set.

test/jdk/java/net/httpclient/CancelRequestTest.java line 497:

> 495:                         out.println(now() + "cancelled " + cf1);
> 496:                     };
> 497:                     if (async) Thread.ofVirtual().start(cancel);

Instead of involving virtual threads — which, IMHO, enlarges the ground for potential problems — can't we use `Executors.newThreadPerTaskExecutor` while initializing `executor`?

test/jdk/java/net/httpclient/CancelRequestTest.java line 790:

> 788:                         out.printf(now() + "Server wrote %d bytes%n", req.length);
> 789:                     }
> 790:                     if (sem != null) {

Can `sem` ever be null?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29415#discussion_r2738420131
PR Review Comment: https://git.openjdk.org/jdk/pull/29415#discussion_r2738323038
PR Review Comment: https://git.openjdk.org/jdk/pull/29415#discussion_r2738370457
PR Review Comment: https://git.openjdk.org/jdk/pull/29415#discussion_r2738428108