From c5943bde4c7092f534a73551dad1d3a6d307dee7 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Sat, 3 Dec 2022 12:40:27 +0530 Subject: [PATCH] [java] Add close method to JDK 11 client. Ensure close methods for Http client is called. (#11345) * [java] Add close method to JDK 11 client. Ensure close methods for Http client is called. * [java] Close the underlying websocket for JDK 11 Http Client --- .../org/openqa/selenium/bidi/Connection.java | 5 +++- .../selenium/devtools/CdpEndpointFinder.java | 3 +-- .../openqa/selenium/devtools/Connection.java | 6 +++-- .../openqa/selenium/devtools/DevTools.java | 1 + .../selenium/remote/RemoteWebDriver.java | 4 ++++ .../remote/http/jdk/JdkHttpClient.java | 23 ++++++++++++++++--- 6 files changed, 34 insertions(+), 8 deletions(-) diff --git a/java/src/org/openqa/selenium/bidi/Connection.java b/java/src/org/openqa/selenium/bidi/Connection.java index dfacb3ebe3647..bb543b1cde7e2 100644 --- a/java/src/org/openqa/selenium/bidi/Connection.java +++ b/java/src/org/openqa/selenium/bidi/Connection.java @@ -71,12 +71,14 @@ public class Connection implements Closeable { private final Map>> methodCallbacks = new ConcurrentHashMap<>(); private final ReadWriteLock callbacksLock = new ReentrantReadWriteLock(true); private final Multimap, Consumer> eventCallbacks = HashMultimap.create(); + private final HttpClient client; public Connection(HttpClient client, String url) { Require.nonNull("HTTP client", client); Require.nonNull("URL to connect to", url); - socket = client.openSocket(new HttpRequest(GET, url), new Listener()); + this.client = client; + socket = this.client.openSocket(new HttpRequest(GET, url), new Listener()); } private static class NamedConsumer implements Consumer { @@ -207,6 +209,7 @@ public void clearListeners() { @Override public void close() { socket.close(); + client.close(); } private class Listener implements WebSocket.Listener { diff --git a/java/src/org/openqa/selenium/devtools/CdpEndpointFinder.java b/java/src/org/openqa/selenium/devtools/CdpEndpointFinder.java index 3c8e38b18ba99..f2713aacc7488 100644 --- a/java/src/org/openqa/selenium/devtools/CdpEndpointFinder.java +++ b/java/src/org/openqa/selenium/devtools/CdpEndpointFinder.java @@ -47,10 +47,9 @@ public static Optional getCdpEndPoint(HttpClient.Factory clientFactory, URI Require.nonNull("DevTools URI", reportedUri); ClientConfig config = ClientConfig.defaultConfig().baseUri(reportedUri); - HttpClient client = clientFactory.createClient(config); HttpResponse res; - try { + try (HttpClient client = clientFactory.createClient(config)) { res = client.execute(new HttpRequest(GET, "/json/version")); } catch (UncheckedIOException e) { LOG.warning("Unable to connect to determine websocket url: " + e.getMessage()); diff --git a/java/src/org/openqa/selenium/devtools/Connection.java b/java/src/org/openqa/selenium/devtools/Connection.java index 0201659d009c3..bd67046897803 100644 --- a/java/src/org/openqa/selenium/devtools/Connection.java +++ b/java/src/org/openqa/selenium/devtools/Connection.java @@ -69,12 +69,13 @@ public class Connection implements Closeable { private final Map>> methodCallbacks = new ConcurrentHashMap<>(); private final ReadWriteLock callbacksLock = new ReentrantReadWriteLock(true); private final Multimap, Consumer> eventCallbacks = HashMultimap.create(); + private final HttpClient client; public Connection(HttpClient client, String url) { Require.nonNull("HTTP client", client); Require.nonNull("URL to connect to", url); - - socket = client.openSocket(new HttpRequest(GET, url), new Listener()); + this.client = client; + socket = this.client.openSocket(new HttpRequest(GET, url), new Listener()); } private static class NamedConsumer implements Consumer { @@ -188,6 +189,7 @@ public void clearListeners() { @Override public void close() { socket.close(); + client.close(); } private class Listener implements WebSocket.Listener { diff --git a/java/src/org/openqa/selenium/devtools/DevTools.java b/java/src/org/openqa/selenium/devtools/DevTools.java index 8febd9b79e966..ba6ea4d7851c4 100644 --- a/java/src/org/openqa/selenium/devtools/DevTools.java +++ b/java/src/org/openqa/selenium/devtools/DevTools.java @@ -58,6 +58,7 @@ public Domains getDomains() { @Override public void close() { disconnectSession(); + connection.close(); } public void disconnectSession() { diff --git a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java index 517bd3554089d..ab1afab8c02a8 100644 --- a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java +++ b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java @@ -440,6 +440,10 @@ public void quit() { } try { + if (this instanceof HasDevTools) { + ((HasDevTools) this).maybeGetDevTools().ifPresent(DevTools::close); + } + execute(DriverCommand.QUIT); } finally { sessionId = null; diff --git a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java index 7d5312ffcd32a..47e378a07dc28 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -55,6 +55,8 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import java.util.logging.Level; @@ -65,7 +67,9 @@ public class JdkHttpClient implements HttpClient { public static final Logger LOG = Logger.getLogger(JdkHttpClient.class.getName()); private final JdkHttpMessages messages; - private final java.net.http.HttpClient client; + private java.net.http.HttpClient client; + private WebSocket websocket; + private final ExecutorService executorService; private final Duration readTimeout; JdkHttpClient(ClientConfig config) { @@ -74,9 +78,12 @@ public class JdkHttpClient implements HttpClient { this.messages = new JdkHttpMessages(config); this.readTimeout = config.readTimeout(); + executorService = Executors.newCachedThreadPool(); + java.net.http.HttpClient.Builder builder = java.net.http.HttpClient.newBuilder() .connectTimeout(config.connectionTimeout()) - .followRedirects(ALWAYS); + .followRedirects(ALWAYS) + .executor(executorService); Credentials credentials = config.credentials(); String info = config.baseUri().getUserInfo(); @@ -196,7 +203,7 @@ public void onError(java.net.http.WebSocket webSocket, Throwable error) { java.net.http.WebSocket underlyingSocket = webSocketCompletableFuture.join(); - return new WebSocket() { + this.websocket = new WebSocket() { @Override public WebSocket send(Message message) { Supplier> makeCall; @@ -252,6 +259,7 @@ public void close() { } } }; + return this.websocket; } private URI getWebSocketUri(HttpRequest request) { @@ -295,6 +303,15 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException { } + @Override + public void close() { + executorService.shutdownNow(); + if (this.websocket != null) { + this.websocket.close(); + } + this.client = null; + } + @AutoService(HttpClient.Factory.class) @HttpClientName("jdk-http-client") public static class Factory implements HttpClient.Factory {