stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8297200: java/net/httpclient/SpecialHeadersTest.java failed once in AssertionError due to selector thread remaining alive

Browse Source 
Reviewed-by: jpai
This commit is contained in:
Daniel Fuchs 2022-11-29 12:42:37 +00:00
parent 5d2772a43e
commit d83a07b72c
4 changed files with 103 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java
View File

@ -359,6 +359,7 @@ final class HttpClientImpl extends HttpClient implements Trackable {
// nature of the API, we also need to wait until all pending operations
// have completed.
private final WeakReference<HttpClientFacade> facadeRef;
private final WeakReference<HttpClientImpl> implRef;
private final ConcurrentSkipListSet<PlainHttpConnection> openedConnections
= new ConcurrentSkipListSet<>(HttpConnection.COMPARE_BY_ID);
@ -402,6 +403,7 @@ final class HttpClientImpl extends HttpClient implements Trackable {
private final AtomicLong pendingTCPConnectionCount = new AtomicLong();
private final AtomicLong pendingSubscribersCount = new AtomicLong();
private final AtomicBoolean isAlive = new AtomicBoolean();
private final AtomicBoolean isStarted = new AtomicBoolean();
/** A Set of, deadline first, ordered timeout events. */
private final TreeSet<TimeoutEvent> timeouts;
@ -470,6 +472,7 @@ final class HttpClientImpl extends HttpClient implements Trackable {
delegatingExecutor = new DelegatingExecutor(this::isSelectorThread, ex,
this::onSubmitFailure);
facadeRef = new WeakReference<>(facadeFactory.createFacade(this));
implRef = new WeakReference<>(this);
client2 = new Http2ClientImpl(this);
cookieHandler = builder.cookieHandler;
connectTimeout = builder.connectTimeout;
@ -519,7 +522,12 @@ final class HttpClientImpl extends HttpClient implements Trackable {
}
private void start() {
selmgr.start();
try {
selmgr.start();
} catch (Throwable t) {
isStarted.set(true);
throw t;
}
}
// Called from the SelectorManager thread, just before exiting.
@ -714,7 +722,9 @@ final class HttpClientImpl extends HttpClient implements Trackable {
final AtomicLong connnectionsCount;
final AtomicLong subscribersCount;
final Reference<?> reference;
final Reference<?> implRef;
final AtomicBoolean isAlive;
final AtomicBoolean isStarted;
final String name;
HttpClientTracker(AtomicLong request,
AtomicLong http,
@ -724,7 +734,9 @@ final class HttpClientImpl extends HttpClient implements Trackable {
AtomicLong conns,
AtomicLong subscribers,
Reference<?> ref,
Reference<?> implRef,
AtomicBoolean isAlive,
AtomicBoolean isStarted,
String name) {
this.requestCount = request;
this.httpCount = http;
@ -734,7 +746,9 @@ final class HttpClientImpl extends HttpClient implements Trackable {
this.connnectionsCount = conns;
this.subscribersCount = subscribers;
this.reference = ref;
this.implRef = implRef;
this.isAlive = isAlive;
this.isStarted = isStarted;
this.name = name;
}
@Override
@ -765,8 +779,12 @@ final class HttpClientImpl extends HttpClient implements Trackable {
public boolean isFacadeReferenced() {
return !reference.refersTo(null);
}
public boolean isImplementationReferenced() {
return !implRef.refersTo(null);
}
// The selector is considered alive if it's not yet started
@Override
public boolean isSelectorAlive() { return isAlive.get(); }
public boolean isSelectorAlive() { return isAlive.get() || !isStarted.get(); }
@Override
public String getName() {
return name;
@ -783,7 +801,9 @@ final class HttpClientImpl extends HttpClient implements Trackable {
pendingTCPConnectionCount,
pendingSubscribersCount,
facadeRef,
implRef,
isAlive,
isStarted,
dbgTag);
}
@ -1155,7 +1175,8 @@ final class HttpClientImpl extends HttpClient implements Trackable {
List<Pair<AsyncEvent,IOException>> errorList = new ArrayList<>();
List<AsyncEvent> readyList = new ArrayList<>();
List<Runnable> resetList = new ArrayList<>();
owner.isAlive.set(true);
owner.isAlive.set(true); // goes back to false when run exits
owner.isStarted.set(true); // never goes back to false
try {
if (Log.channel()) Log.logChannel(getName() + ": starting");
while (!Thread.currentThread().isInterrupted() && !closed) {

3
src/java.net.http/share/classes/jdk/internal/net/http/common/OperationTrackers.java
View File

@ -64,6 +64,9 @@ public final class OperationTrackers {
// Whether the facade returned to the
// user is still referenced
boolean isFacadeReferenced();
// Whether the implementation of the facade
// is still referenced
boolean isImplementationReferenced();
// whether the Selector Manager thread is still running
boolean isSelectorAlive();
// The name of the object being tracked.

27
test/jdk/java/net/httpclient/ReferenceTracker.java
View File

@ -30,6 +30,7 @@ import java.lang.management.ManagementFactory;
import java.lang.management.MonitorInfo;
import java.lang.management.ThreadInfo;
import java.net.http.HttpClient;
import java.time.Duration;
import java.util.Arrays;
import java.util.Objects;
import java.util.concurrent.ConcurrentLinkedQueue;
@ -103,7 +104,7 @@ public class ReferenceTracker {
hasOperations.or(hasSubscribers)
.or(Tracker::isFacadeReferenced)
.or(Tracker::isSelectorAlive),
"outstanding operations or unreleased resources", false);
"outstanding operations or unreleased resources", true);
}
public AssertionError check(long graceDelayMs) {
@ -210,6 +211,9 @@ public class ReferenceTracker {
graceDelayMs = Math.max(graceDelayMs, 100);
long delay = Math.min(graceDelayMs, 10);
var count = delay > 0 ? graceDelayMs / delay : 1;
long waitStart = System.nanoTime();
long waited = 0;
long toWait = Math.min(graceDelayMs, Math.max(delay, 1));
for (int i = 0; i < count; i++) {
if (hasOutstanding.test(tracker)) {
System.gc();
@ -217,25 +221,36 @@ public class ReferenceTracker {
if (i == 0) {
System.out.println("Waiting for HTTP operations to terminate...");
}
Thread.sleep(Math.min(graceDelayMs, Math.max(delay, 1)));
waited += toWait;
Thread.sleep(toWait);
} catch (InterruptedException x) {
// OK
}
} else break;
} else {
System.out.println("No outstanding HTTP operations remaining after "
+ i + "/" + count + " iterations and " + waited + "/" + graceDelayMs
+ " ms, (wait/iteration " + toWait + " ms)");
break;
}
}
long duration = Duration.ofNanos(System.nanoTime() - waitStart).toMillis();
if (hasOutstanding.test(tracker)) {
StringBuilder warnings = diagnose(tracker, new StringBuilder(), hasOutstanding);
if (hasOutstanding.test(tracker)) {
fail = new AssertionError(warnings.toString());
}
} else {
System.out.println("PASSED: No " + description + " found in " + tracker.getName());
System.out.println("PASSED: No " + description + " found in "
+ tracker.getName() + " in " + duration + " ms");
}
if (fail != null) {
if (printThreads && tracker.isSelectorAlive()) {
printThreads("Some selector manager threads are still alive: ", System.out);
printThreads("Some selector manager threads are still alive: ", System.err);
var msg = "Selector manager threads are still alive for " + tracker.getName() + ": ";
printThreads(msg, System.out);
printThreads(msg, System.err);
}
System.out.println("AssertionError: Found some " + description + " in "
+ tracker.getName() + " after " + duration + " ms, waited " + waited + " ms");
}
return fail;
}

70
test/jdk/java/net/httpclient/SpecialHeadersTest.java
View File

@ -25,7 +25,7 @@
* @test
* @summary Verify that some special headers - such as User-Agent
* can be specified by the caller.
* @bug 8203771 8218546
* @bug 8203771 8218546 8297200
* @modules java.base/sun.net.www.http
* java.net.http/jdk.internal.net.http.common
* java.net.http/jdk.internal.net.http.frame
@ -46,6 +46,7 @@
import com.sun.net.httpserver.HttpServer;
import com.sun.net.httpserver.HttpsConfigurator;
import com.sun.net.httpserver.HttpsServer;
import jdk.internal.net.http.common.OperationTrackers.Tracker;
import jdk.test.lib.net.SimpleSSLContext;
import org.testng.ITestContext;
import org.testng.ITestResult;
@ -301,13 +302,18 @@ public class SpecialHeadersTest implements HttpServerAdapters {
static final Map<String, Function<URI,String>> DEFAULTS = Map.of(
"USER-AGENT", u -> userAgent(), "HOST", u -> u.getRawAuthority());
static void throwIfNotNull(Throwable throwable) throws Exception {
if (throwable instanceof Exception ex) throw ex;
if (throwable instanceof Error e) throw e;
}
@Test(dataProvider = "variants")
void test(String uriString,
String headerNameAndValue,
boolean sameClient)
throws Exception
{
out.println("\n--- Starting ");
out.println("\n--- Starting test " + now());
int index = headerNameAndValue.indexOf(":");
String name = headerNameAndValue.substring(0, index);
@ -319,10 +325,14 @@ public class SpecialHeadersTest implements HttpServerAdapters {
String value = useDefault ? DEFAULTS.get(key).apply(uri) : v;
HttpClient client = null;
Tracker tracker = null;
Throwable thrown = null;
for (int i=0; i< ITERATION_COUNT; i++) {
try {
if (!sameClient || client == null)
if (!sameClient || client == null) {
client = newHttpClient("test", sameClient);
tracker = TRACKER.getTracker(client);
}
HttpRequest.Builder requestBuilder = HttpRequest.newBuilder(uri);
if (!useDefault) {
@ -362,14 +372,20 @@ public class SpecialHeadersTest implements HttpServerAdapters {
assertEquals(resp.headers().allValues("X-" + key).size(), 0);
}
}
} catch (Throwable x) {
thrown = x;
} finally {
if (!sameClient) {
client = null;
System.gc();
var error = TRACKER.check(500);
if (error != null) throw error;
var error = TRACKER.check(tracker, 500);
if (error != null) {
if (thrown != null) error.addSuppressed(thrown);
throw error;
}
}
}
throwIfNotNull(thrown);
}
}
@ -379,11 +395,12 @@ public class SpecialHeadersTest implements HttpServerAdapters {
boolean sameClient)
throws Exception
{
out.println("\n--- Starting ");
out.println("\n--- Starting testHomeMadeIllegalHeader " + now());
final URI uri = URI.create(uriString);
HttpClient client = newHttpClient("testHomeMadeIllegalHeader", sameClient);
Tracker tracker = TRACKER.getTracker(client);
Throwable thrown = null;
try {
// Test a request which contains an illegal header created
HttpRequest req = new HttpRequest() {
@ -430,19 +447,29 @@ public class SpecialHeadersTest implements HttpServerAdapters {
} catch (IllegalArgumentException ee) {
out.println("Got IAE as expected");
}
} catch (Throwable x) {
thrown = x;
} finally {
if (!sameClient) {
client = null;
System.gc();
var error = TRACKER.check(500);
if (error != null) throw error;
var error = TRACKER.check(tracker, 500);
if (error != null) {
if (thrown != null) error.addSuppressed(thrown);
throw error;
}
}
}
throwIfNotNull(thrown);
}
@Test(dataProvider = "variants")
void testAsync(String uriString, String headerNameAndValue, boolean sameClient) {
out.println("\n--- Starting ");
void testAsync(String uriString, String headerNameAndValue, boolean sameClient)
throws Exception
{
out.println("\n--- Starting testAsync " + now());
int index = headerNameAndValue.indexOf(":");
String name = headerNameAndValue.substring(0, index);
String v = headerNameAndValue.substring(index+1).trim();
@ -453,10 +480,14 @@ public class SpecialHeadersTest implements HttpServerAdapters {
String value = useDefault ? DEFAULTS.get(key).apply(uri) : v;
HttpClient client = null;
Tracker tracker = null;
Throwable thrown = null;
for (int i=0; i< ITERATION_COUNT; i++) {
try {
if (!sameClient || client == null)
if (!sameClient || client == null) {
client = newHttpClient("testAsync", sameClient);
tracker = TRACKER.getTracker(client);
}
HttpRequest.Builder requestBuilder = HttpRequest.newBuilder(uri);
if (!useDefault) {
@ -499,14 +530,20 @@ public class SpecialHeadersTest implements HttpServerAdapters {
}
})
.join();
} catch (Throwable x) {
thrown = x;
} finally {
if (!sameClient) {
client = null;
System.gc();
var error = TRACKER.check(500);
if (error != null) throw error;
var error = TRACKER.check(tracker, 500);
if (error != null) {
if (thrown != null) error.addSuppressed(thrown);
throw error;
}
}
}
throwIfNotNull(thrown);
}
}
@ -517,6 +554,7 @@ public class SpecialHeadersTest implements HttpServerAdapters {
@BeforeTest
public void setup() throws Exception {
out.println("--- Starting setup " + now());
sslContext = new SimpleSSLContext().get();
if (sslContext == null)
throw new AssertionError("Unexpected null sslContext");
@ -549,13 +587,15 @@ public class SpecialHeadersTest implements HttpServerAdapters {
@AfterTest
public void teardown() throws Exception {
out.println("\n--- Teardown " + now());
HttpClient shared = sharedClient;
String sharedClientName =
shared == null ? null : shared.toString();
if (shared != null) TRACKER.track(shared);
shared = sharedClient = null;
Thread.sleep(100);
AssertionError fail = TRACKER.check(500);
AssertionError fail = TRACKER.check(2500);
out.println("--- Stopping servers " + now());
try {
httpTestServer.stop();
httpsTestServer.stop();