Skip to content
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

[java] Ensure calling close() and quit() done cause BiDi websocket errors #13333

Merged
merged 4 commits into from
Dec 20, 2023
Merged
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
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 tries to close an already closed websocket and that
// causes errors.
// Ideally, such errors should not prevent freeing up resources.
// This measure is needed 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
Loading