8380967: Canceled HttpClient.sendAsync futures throw inconsistent exceptions

Reviewed-by: dfuchs
This commit is contained in:
Benjamin Peterson 2026-07-09 06:57:14 +00:00 committed by Daniel Fuchs
parent 05c93a1dbb
commit 333deb2cc6
3 changed files with 48 additions and 26 deletions

View File

@ -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<T> extends CompletableFuture<T> {
@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() {

View File

@ -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());

View File

@ -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<Object> 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<Object> otherFuture() {
return MinimalFuture.completedFuture(new Object());
}