From 333deb2cc63d8a16600a09e5f933c2a5091fda3d Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Thu, 9 Jul 2026 06:57:14 +0000 Subject: [PATCH] 8380967: Canceled HttpClient.sendAsync futures throw inconsistent exceptions Reviewed-by: dfuchs --- .../net/http/common/MinimalFuture.java | 13 +++++--- .../net/httpclient/CancelRequestTest.java | 29 +++++------------ .../net/http/common/MinimalFutureTest.java | 32 +++++++++++++++++++ 3 files changed, 48 insertions(+), 26 deletions(-) diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/common/MinimalFuture.java b/src/java.net.http/share/classes/jdk/internal/net/http/common/MinimalFuture.java index ddbcce661aa..268705f6c1f 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/common/MinimalFuture.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/common/MinimalFuture.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -102,11 +102,14 @@ public final class MinimalFuture extends CompletableFuture { @Override public boolean cancel(boolean mayInterruptIfRunning) { - boolean result = false; - if (cancelable != null && !isDone()) { - result = cancelable.cancel(mayInterruptIfRunning); + if (!super.cancel(mayInterruptIfRunning)) { + assert isDone(); + return false; } - return super.cancel(mayInterruptIfRunning) || result; + if (cancelable != null) { + cancelable.cancel(mayInterruptIfRunning); + } + return true; } private Cancelable cancelable() { diff --git a/test/jdk/java/net/httpclient/CancelRequestTest.java b/test/jdk/java/net/httpclient/CancelRequestTest.java index f21d13d5e98..418127c7735 100644 --- a/test/jdk/java/net/httpclient/CancelRequestTest.java +++ b/test/jdk/java/net/httpclient/CancelRequestTest.java @@ -23,7 +23,7 @@ /* * @test - * @bug 8245462 8229822 8254786 8297075 8297149 8298340 8302635 8377181 + * @bug 8245462 8229822 8254786 8297075 8297149 8298340 8302635 8377181 8380967 * @summary Tests cancelling the request. * @library /test/lib /test/jdk/java/net/httpclient/lib * @key randomness @@ -79,6 +79,7 @@ import org.junit.jupiter.api.AfterAll; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.Assumptions; @@ -394,15 +395,9 @@ public class CancelRequestTest implements HttpServerAdapters { requestLatch.countDown(); } - // Cancelling the request may cause an IOException instead... - boolean hasCancellationException = false; - try { - cf1.get(); - } catch (CancellationException | ExecutionException x) { - out.println(now() + "Got expected exception: " + x); - assertTrue(isCancelled(x)); - hasCancellationException = x instanceof CancellationException; - } + var cancelX = assertThrows(CancellationException.class, cf1::get); + out.println(now() + "Got expected exception: " + cancelX); + assertTrue(cf1.isCancelled()); // because it's cf1 that was cancelled then response might not have // completed yet - so wait for it here... @@ -447,7 +442,6 @@ public class CancelRequestTest implements HttpServerAdapters { assertTrue(response.isDone()); assertFalse(response.isCancelled()); - assertEquals(hasCancellationException, cf1.isCancelled()); assertTrue(cf2.isDone()); assertFalse(cf2.isCancelled()); assertEquals(0, latch.getCount()); @@ -529,15 +523,9 @@ public class CancelRequestTest implements HttpServerAdapters { requestLatch.countDown(); } - // Cancelling the request may cause an IOException instead... - boolean hasCancellationException = false; - try { - cf1.get(); - } catch (CancellationException | ExecutionException x) { - out.println(now() + "Got expected exception: " + x); - assertTrue(isCancelled(x)); - hasCancellationException = x instanceof CancellationException; - } + var cancelX = assertThrows(CancellationException.class, cf1::get); + out.println(now() + "Got expected exception: " + cancelX); + assertTrue(cf1.isCancelled()); // because it's cf1 that was cancelled then response might not have // completed yet - so wait for it here... @@ -576,7 +564,6 @@ public class CancelRequestTest implements HttpServerAdapters { assertTrue(response.isDone()); assertFalse(response.isCancelled()); - assertEquals(hasCancellationException, cf1.isCancelled()); assertTrue(cf2.isDone()); assertFalse(cf2.isCancelled()); assertEquals(0, latch.getCount()); diff --git a/test/jdk/java/net/httpclient/whitebox/java.net.http/jdk/internal/net/http/common/MinimalFutureTest.java b/test/jdk/java/net/httpclient/whitebox/java.net.http/jdk/internal/net/http/common/MinimalFutureTest.java index 2c33f6f0018..ccac8fcf439 100644 --- a/test/jdk/java/net/httpclient/whitebox/java.net.http/jdk/internal/net/http/common/MinimalFutureTest.java +++ b/test/jdk/java/net/httpclient/whitebox/java.net.http/jdk/internal/net/http/common/MinimalFutureTest.java @@ -27,10 +27,16 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; public class MinimalFutureTest { @@ -101,6 +107,32 @@ public class MinimalFutureTest { } } + @Test + public void testCancel() { + AtomicInteger cancelCount = new AtomicInteger(); + AtomicBoolean cancelled = new AtomicBoolean(); + Cancelable cancelable = mayInterruptIfRunning -> { + cancelCount.incrementAndGet(); + if (mayInterruptIfRunning) { + cancelled.set(true); + } + return cancelled.get(); + }; + MinimalFuture future = new MinimalFuture<>(cancelable); + CompletableFuture dependent = future.copy().whenComplete((x,t) -> + System.out.println("expected: " + t)); + assertTrue(dependent.cancel(false)); + assertTrue(dependent.isCancelled()); + assertFalse(future.isCancelled()); + assertFalse(cancelled.get()); + assertEquals(1, cancelCount.get()); + assertTrue(dependent.cancel(true)); + assertTrue(dependent.isCancelled()); + assertFalse(future.isCancelled()); + assertTrue(cancelled.get()); + assertEquals(2, cancelCount.get()); + } + private static CompletableFuture otherFuture() { return MinimalFuture.completedFuture(new Object()); }