Skip to content

Commit

Permalink
[java] Add close method to JDK 11 client. Ensure close methods for Ht…
Browse files Browse the repository at this point in the history
…tp 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
  • Loading branch information
pujagani authored Dec 3, 2022
1 parent 630fc47 commit c5943bd
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 8 deletions.
5 changes: 4 additions & 1 deletion java/src/org/openqa/selenium/bidi/Connection.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,14 @@ public class Connection implements Closeable {
private final Map<Long, Consumer<Either<Throwable, JsonInput>>> methodCallbacks = new ConcurrentHashMap<>();
private final ReadWriteLock callbacksLock = new ReentrantReadWriteLock(true);
private final Multimap<Event<?>, 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<X> implements Consumer<X> {
Expand Down Expand Up @@ -207,6 +209,7 @@ public void clearListeners() {
@Override
public void close() {
socket.close();
client.close();
}

private class Listener implements WebSocket.Listener {
Expand Down
3 changes: 1 addition & 2 deletions java/src/org/openqa/selenium/devtools/CdpEndpointFinder.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,9 @@ public static Optional<URI> 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());
Expand Down
6 changes: 4 additions & 2 deletions java/src/org/openqa/selenium/devtools/Connection.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,13 @@ public class Connection implements Closeable {
private final Map<Long, Consumer<Either<Throwable, JsonInput>>> methodCallbacks = new ConcurrentHashMap<>();
private final ReadWriteLock callbacksLock = new ReentrantReadWriteLock(true);
private final Multimap<Event<?>, 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<X> implements Consumer<X> {
Expand Down Expand Up @@ -188,6 +189,7 @@ public void clearListeners() {
@Override
public void close() {
socket.close();
client.close();
}

private class Listener implements WebSocket.Listener {
Expand Down
1 change: 1 addition & 0 deletions java/src/org/openqa/selenium/devtools/DevTools.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public Domains getDomains() {
@Override
public void close() {
disconnectSession();
connection.close();
}

public void disconnectSession() {
Expand Down
4 changes: 4 additions & 0 deletions java/src/org/openqa/selenium/remote/RemoteWebDriver.java
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,10 @@ public void quit() {
}

try {
if (this instanceof HasDevTools) {
((HasDevTools) this).maybeGetDevTools().ifPresent(DevTools::close);
}

execute(DriverCommand.QUIT);
} finally {
sessionId = null;
Expand Down
23 changes: 20 additions & 3 deletions java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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();
Expand Down Expand Up @@ -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<CompletableFuture<java.net.http.WebSocket>> makeCall;
Expand Down Expand Up @@ -252,6 +259,7 @@ public void close() {
}
}
};
return this.websocket;
}

private URI getWebSocketUri(HttpRequest request) {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit c5943bd

Please sign in to comment.