Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
import jdk.internal.net.http.common.OperationTrackers.Tracker;
import jdk.internal.net.http.common.Utils.SafeExecutor;
import jdk.internal.net.http.common.Utils.SafeExecutorService;
import jdk.internal.net.http.common.Utils.UseVTForSelector;
import jdk.internal.net.http.websocket.BuilderImpl;

import static java.net.http.HttpOption.Http3DiscoveryMode.HTTP_3_URI_ONLY;
Expand Down Expand Up @@ -126,6 +127,16 @@ final class HttpClientImpl extends HttpClient implements Trackable {
static final long IDLE_CONNECTION_TIMEOUT_H2 = getTimeoutProp("jdk.httpclient.keepalive.timeout.h2", KEEP_ALIVE_TIMEOUT);
static final long IDLE_CONNECTION_TIMEOUT_H3 = getTimeoutProp("jdk.httpclient.keepalive.timeout.h3", IDLE_CONNECTION_TIMEOUT_H2);

static final UseVTForSelector USE_VT_FOR_SELECTOR =
Utils.useVTForSelector("jdk.internal.httpclient.tcp.selector.useVirtualThreads", "default");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello Daniel, I vaguely remember we discussed whether or not to use a value called "default" for this property. But I don't remember what we decided. Looking at this now, I am wondering whether we should just do something like:

System.getProperty("jdk.internal.httpclient.tcp.selector.useVirtualThreads", "true")

to keep it simple?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For tcp we only need true/false - but for quic we have a default behavior which depends on the platform. I decided to keep "always/never/default" for both properties for consistency. Let me think about it. I am not sure if that change (true/false for quic too) should be made in this PR.

private static boolean useVtForSelector() {
return switch (USE_VT_FOR_SELECTOR) {
case ALWAYS -> true;
case NEVER -> false;
default -> true;
};
}

// Define the default factory as a static inner class
// that embeds all the necessary logic to avoid
// the risk of using a lambda that might keep a reference on the
Expand Down Expand Up @@ -293,7 +304,6 @@ static <T> CompletableFuture<T> registerPending(PendingRequest pending, Completa
if (pending.cf.isDone()) return res;

var client = pending.client;
var cf = pending.cf;
var id = pending.id;
boolean added = client.pendingRequests.add(pending);
// this may immediately remove `pending` from the set is the cf is already completed
Expand Down Expand Up @@ -342,6 +352,7 @@ static void abortPendingRequests(HttpClientImpl client, Throwable reason) {
private final boolean isDefaultExecutor;
private final SSLContext sslContext;
private final SSLParameters sslParams;
private final Thread selmgrThread;
private final SelectorManager selmgr;
private final FilterFactory filters;
private final Http2ClientImpl client2;
Expand Down Expand Up @@ -509,7 +520,11 @@ private HttpClientImpl(HttpClientBuilderImpl builder,
// unlikely
throw new UncheckedIOException(e);
}
selmgr.setDaemon(true);
selmgrThread = useVtForSelector()
? Thread.ofVirtual().name("HttpClient-" + id + "-SelectorManager")
.inheritInheritableThreadLocals(false).unstarted(selmgr)
: Thread.ofPlatform().name("HttpClient-" + id + "-SelectorManager")
.inheritInheritableThreadLocals(false).daemon().unstarted(selmgr);
filters = new FilterFactory();
initFilters();
assert facadeRef.get() != null;
Expand All @@ -528,7 +543,7 @@ void onSubmitFailure(Runnable command, Throwable failure) {

private void start() {
try {
selmgr.start();
selmgrThread.start();
} catch (Throwable t) {
isStarted.set(true);
throw t;
Expand Down Expand Up @@ -635,7 +650,7 @@ public void shutdownNow() {
@Override
public boolean awaitTermination(Duration duration) throws InterruptedException {
// Implicit NPE will be thrown if duration is null
return selmgr.join(duration);
return selmgrThread.join(duration);
}

@Override
Expand Down Expand Up @@ -927,7 +942,7 @@ void eventUpdated(AsyncEvent event) throws ClosedChannelException {
}

boolean isSelectorThread() {
return Thread.currentThread() == selmgr;
return Thread.currentThread() == selmgrThread;
}

AltServicesRegistry registry() { return registry; }
Expand Down Expand Up @@ -1157,7 +1172,7 @@ private static <T> CompletableFuture<HttpResponse<T>> translateSendAsyncExecFail
}

// Main loop for this client's selector
private static final class SelectorManager extends Thread {
private static final class SelectorManager implements Runnable {

// For testing purposes we have an internal System property that
// can control the frequency at which the selector manager will wake
Expand Down Expand Up @@ -1196,9 +1211,6 @@ private static final class SelectorManager extends Thread {
private final ReentrantLock lock = new ReentrantLock();

SelectorManager(HttpClientImpl ref) throws IOException {
super(null, null,
"HttpClient-" + ref.id + "-SelectorManager",
0, false);
owner = ref;
debug = ref.debug;
debugtimeout = ref.debugtimeout;
Expand All @@ -1221,7 +1233,7 @@ IOException selectorClosedException() {
}

void eventUpdated(AsyncEvent e) throws ClosedChannelException {
if (Thread.currentThread() == this) {
if (owner.isSelectorThread()) {
SelectionKey key = e.channel().keyFor(selector);
if (key != null && key.isValid()) {
SelectorAttachment sa = (SelectorAttachment) key.attachment();
Expand Down Expand Up @@ -1315,6 +1327,10 @@ void abort(Throwable t) {
if (!inSelectorThread) selector.wakeup();
}

String getName() {
return owner.selmgrThread.getName();
}

// Only called by the selector manager thread
private void shutdown() {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,15 @@ private static Set<String> getDisallowedRedirectHeaders() {
return true;
};

public enum UseVTForSelector { ALWAYS, NEVER, DEFAULT }

public static UseVTForSelector useVTForSelector(String property, String defval) {
String useVtForSelector = System.getProperty(property, defval);
return Stream.of(UseVTForSelector.values())
.filter((v) -> v.name().equalsIgnoreCase(useVtForSelector))
.findFirst().orElse(UseVTForSelector.DEFAULT);
}

public static <T extends Throwable> T addSuppressed(T x, Throwable suppressed) {
if (x != suppressed && suppressed != null) {
var sup = x.getSuppressed();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,9 @@ void incomingStatelessReset() {
Log.logError("{0}: stateless reset from peer ({1})", connection.logTag(),
(peerIsServer ? "server" : "client"));
}
var label = "quic:" + connection.uniqueId();
final SilentTermination st = forSilentTermination("stateless reset from peer ("
+ (peerIsServer ? "server" : "client") + ")");
+ (peerIsServer ? "server" : "client") + ") on " + label);
terminate(st);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,10 @@ private void idleTimedOut() {
}
}
// silently close the connection and discard all its state
final TerminationCause cause = forSilentTermination("connection idle timed out ("
+ timeoutMillis + " milli seconds)");
var type = connection.isClientConnection() ? "client" : "server";
var label = "quic:" + connection.uniqueId();
final TerminationCause cause = forSilentTermination(type + " connection idle timed out ("
+ timeoutMillis + " milli seconds) on " + label);
connection.terminator.terminate(cause);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import jdk.internal.net.http.common.TimeLine;
import jdk.internal.net.http.common.TimeSource;
import jdk.internal.net.http.common.Utils;
import jdk.internal.net.http.common.Utils.UseVTForSelector;
import jdk.internal.net.http.quic.QuicSelector.QuicNioSelector;
import jdk.internal.net.http.quic.QuicSelector.QuicVirtualThreadPoller;
import jdk.internal.net.http.quic.packets.QuicPacket.HeadersType;
Expand Down Expand Up @@ -116,7 +117,6 @@ public abstract sealed class QuicEndpoint implements AutoCloseable
static final boolean DGRAM_SEND_ASYNC;
static final int MAX_BUFFERED_HIGH;
static final int MAX_BUFFERED_LOW;
enum UseVTForSelector { ALWAYS, NEVER, DEFAULT }
static final UseVTForSelector USE_VT_FOR_SELECTOR;
static {
// This default value is the maximum payload size of
Expand Down Expand Up @@ -144,11 +144,8 @@ enum UseVTForSelector { ALWAYS, NEVER, DEFAULT }
if (maxBufferLow >= maxBufferHigh) maxBufferLow = maxBufferHigh >> 1;
MAX_BUFFERED_HIGH = maxBufferHigh;
MAX_BUFFERED_LOW = maxBufferLow;
String useVtForSelector =
System.getProperty("jdk.internal.httpclient.quic.selector.useVirtualThreads", "default");
USE_VT_FOR_SELECTOR = Stream.of(UseVTForSelector.values())
.filter((v) -> v.name().equalsIgnoreCase(useVtForSelector))
.findFirst().orElse(UseVTForSelector.DEFAULT);
var property = "jdk.internal.httpclient.quic.selector.useVirtualThreads";
USE_VT_FOR_SELECTOR = Utils.useVTForSelector(property, "default");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@
import jdk.internal.net.http.common.TimeLine;
import jdk.internal.net.http.common.TimeSource;
import jdk.internal.net.http.common.Utils;
import jdk.internal.net.http.common.Utils.UseVTForSelector;
import jdk.internal.net.http.quic.QuicEndpoint.QuicVirtualThreadedEndpoint;
import jdk.internal.net.http.quic.QuicEndpoint.QuicSelectableEndpoint;
import jdk.internal.net.http.quic.QuicEndpoint.UseVTForSelector;

import static jdk.internal.net.http.quic.QuicEndpoint.USE_VT_FOR_SELECTOR;

Expand Down
8 changes: 1 addition & 7 deletions test/jdk/java/net/httpclient/ReferenceTracker.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2025, 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
Expand Down Expand Up @@ -390,12 +390,6 @@ private static void checkOutstandingOperations(StringBuilder warning,
}
}

private boolean isSelectorManager(Thread t) {
String name = t.getName();
if (name == null) return false;
return name.contains("SelectorManager");
}

// This is a slightly more permissive check than the default checks,
// it only verifies that all CFs returned by send/sendAsync have been
// completed, and that all opened channels have been closed, and that
Expand Down
Loading