On Wed, 28 Jan 2026 20:08:45 GMT, Volkan Yazici <[email protected]> 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.
Good idea. I will have to re-test extensively.
> 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`?
I'd rather use a virtual thread here. The code executed is fairly simple:
interrupt the main thread or cancel a completable future. The alternative is to
revert to using the thread pool executor. As I said the change to using virtual
thread was somewhat gratuitous (eliminated one possible failure reason while
searching for the real root cause).
> 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?
Yes. Client and server run at their own pace. It could be that when the server
reaches here the request has already been cancelled from the client side and
the next test has removed all semaphores from the map before adding its own.
It could also be that the server will never reach here because the stream reset
has already been processed and os.write() has thrown.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29415#discussion_r2740885121
PR Review Comment: https://git.openjdk.org/jdk/pull/29415#discussion_r2740903205
PR Review Comment: https://git.openjdk.org/jdk/pull/29415#discussion_r2740938416
