Skip to content

Commit

Permalink
[java] Ensure calling close() and quit() done cause BiDi websocket er…
Browse files Browse the repository at this point in the history
…rors

Fixes SeleniumHQ#13316
  • Loading branch information
pujagani committed Dec 19, 2023
1 parent 4606e6b commit ac3c932
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 11 deletions.
5 changes: 1 addition & 4 deletions java/src/org/openqa/selenium/bidi/BiDi.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,9 @@ public class BiDi implements Closeable {

private final Duration timeout = Duration.ofSeconds(30);
private final Connection connection;
private final BiDiSessionStatus status;

public BiDi(Connection connection) {
this.connection = Require.nonNull("WebSocket connection", connection);
this.status =
send(new Command<>("session.status", Collections.emptyMap(), BiDiSessionStatus.class));
}

@Override
Expand Down Expand Up @@ -119,6 +116,6 @@ public void clearListeners() {
}

public BiDiSessionStatus getBidiSessionStatus() {
return status;
return send(new Command<>("session.status", Collections.emptyMap(), BiDiSessionStatus.class));
}
}
20 changes: 15 additions & 5 deletions java/src/org/openqa/selenium/bidi/Connection.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,21 +66,28 @@ public class Connection implements Closeable {
return thread;
});
private static final AtomicLong NEXT_ID = new AtomicLong(1L);
private final WebSocket socket;
private WebSocket socket;
private final Map<Long, Consumer<Either<Throwable, JsonInput>>> methodCallbacks =
new ConcurrentHashMap<>();
private final ReadWriteLock callbacksLock = new ReentrantReadWriteLock(true);
private final Map<Event<?>, List<Consumer<?>>> eventCallbacks = new HashMap<>();
private final HttpClient client;
private final AtomicBoolean underlyingSocketClosed;
private final AtomicBoolean underlyingSocketClosed = new AtomicBoolean();

public Connection(HttpClient client, String url) {
Require.nonNull("HTTP client", client);
Require.nonNull("URL to connect to", url);

this.client = client;
socket = this.client.openSocket(new HttpRequest(GET, url), new Listener());
underlyingSocketClosed = new AtomicBoolean();
// If WebDriver close() is called, it closes the session if it is the last browsing context.
// It also closes the WebSocket from the remote end.
// If WebDriver quit() is called, it also tried to close an already closed websocket and that
// causes errors.
// Ideally, such errors should not prevent freeing up resources.
// This measure is need until "session.end" from BiDi is implemented by the browsers.
if (!underlyingSocketClosed.get()) {
socket = this.client.openSocket(new HttpRequest(GET, url), new Listener());
}
}

private static class NamedConsumer<X> implements Consumer<X> {
Expand Down Expand Up @@ -230,7 +237,10 @@ public void clearListeners() {

@Override
public void close() {
socket.close();
if (!underlyingSocketClosed.get()) {
underlyingSocketClosed.set(true);
socket.close();
}
client.close();
}

Expand Down
4 changes: 2 additions & 2 deletions java/src/org/openqa/selenium/manager/SeleniumManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ public Result getDriverPath(Capabilities options, boolean offline) {
List<String> arguments = new ArrayList<>();
arguments.add("--browser");
arguments.add(options.getBrowserName());
arguments.add("--language-binding");
arguments.add("java");
// arguments.add("--language-binding");
// arguments.add("java");
arguments.add("--output");
arguments.add("json");

Expand Down

0 comments on commit ac3c932

Please sign in to comment.