Skip to content

[java] JdkHttpClient - Close all websockets before shutting down the executor #12035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 15, 2023
Merged
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
29 changes: 17 additions & 12 deletions java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import java.net.http.HttpTimeoutException;
import java.nio.ByteBuffer;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.concurrent.CancellationException;
Expand All @@ -67,7 +68,7 @@ public class JdkHttpClient implements HttpClient {
public static final Logger LOG = Logger.getLogger(JdkHttpClient.class.getName());
private final JdkHttpMessages messages;
private java.net.http.HttpClient client;
private WebSocket websocket;
private final List<WebSocket> websockets;
private final ExecutorService executorService;
private final Duration readTimeout;

Expand All @@ -76,7 +77,7 @@ public class JdkHttpClient implements HttpClient {

this.messages = new JdkHttpMessages(config);
this.readTimeout = config.readTimeout();

websockets = new ArrayList<>();
executorService = Executors.newCachedThreadPool();

java.net.http.HttpClient.Builder builder = java.net.http.HttpClient.newBuilder()
Expand Down Expand Up @@ -202,7 +203,7 @@ public void onError(java.net.http.WebSocket webSocket, Throwable error) {

java.net.http.WebSocket underlyingSocket = webSocketCompletableFuture.join();

this.websocket = new WebSocket() {
WebSocket websocket = new WebSocket() {
@Override
public WebSocket send(Message message) {
Supplier<CompletableFuture<java.net.http.WebSocket>> makeCall;
Expand Down Expand Up @@ -259,14 +260,11 @@ public WebSocket send(Message message) {
@Override
public void close() {
LOG.fine("Closing websocket");
synchronized (underlyingSocket) {
if (!underlyingSocket.isOutputClosed()) {
underlyingSocket.sendClose(1000, "WebDriver closing socket");
}
}
send(new CloseMessage(1000, "WebDriver closing socket"));
}
};
return this.websocket;
websockets.add(websocket);
return websocket;
}

private URI getWebSocketUri(HttpRequest request) {
Expand Down Expand Up @@ -347,11 +345,18 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException {

@Override
public void close() {
executorService.shutdownNow();
if (this.websocket != null) {
this.websocket.close();
if (this.client == null) {
return ;
}
this.client = null;
for (WebSocket websocket : websockets) {
try {
websocket.close();
} catch (Exception e) {
LOG.log(Level.WARNING, "failed to close the websocket: " + websocket, e);
}
}
executorService.shutdownNow();
}

@AutoService(HttpClient.Factory.class)
Expand Down