From 100ac7a1bb80d7bffb4740d54f0328a6e58298e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Sautter?= Date: Thu, 17 Sep 2026 17:48:01 +0200 Subject: [PATCH 1/3] [java] Do not leak the HttpClient on startup failure --- .../selenium/remote/HttpCommandExecutor.java | 8 +++++++- .../openqa/selenium/remote/RemoteWebDriver.java | 17 +++++++---------- .../remote/service/DriverCommandExecutor.java | 10 +++++++++- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/java/src/org/openqa/selenium/remote/HttpCommandExecutor.java b/java/src/org/openqa/selenium/remote/HttpCommandExecutor.java index f3ba3e70cb698..5c2ab0e4b4bba 100644 --- a/java/src/org/openqa/selenium/remote/HttpCommandExecutor.java +++ b/java/src/org/openqa/selenium/remote/HttpCommandExecutor.java @@ -23,6 +23,7 @@ import static org.openqa.selenium.remote.DriverCommand.QUIT; import static org.openqa.selenium.remote.HttpSessionId.getSessionId; +import java.io.Closeable; import java.io.IOException; import java.net.URL; import java.util.Collections; @@ -39,7 +40,7 @@ import org.openqa.selenium.remote.http.HttpRequest; import org.openqa.selenium.remote.http.HttpResponse; -public class HttpCommandExecutor implements CommandExecutor { +public class HttpCommandExecutor implements CommandExecutor, Closeable { private final URL remoteServer; protected final Map additionalCommands; @@ -241,4 +242,9 @@ public Response execute(Command command) throws IOException { throw e; } } + + @Override + public void close() { + client.close(); + } } diff --git a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java index 20bd09a9cb273..e5c6ceef11432 100644 --- a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java +++ b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java @@ -98,7 +98,6 @@ import org.openqa.selenium.remote.http.Contents; import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.http.jdk.ConnectionException; -import org.openqa.selenium.remote.service.DriverCommandExecutor; import org.openqa.selenium.remote.tracing.TracedHttpClient; import org.openqa.selenium.remote.tracing.opentelemetry.OpenTelemetryTracer; import org.openqa.selenium.virtualauthenticator.Credential; @@ -320,15 +319,12 @@ protected void startSession(Capabilities capabilities) { sessionId = new SessionId(response.getSessionId()); this.biDi = createBiDi(); } catch (Exception e) { - // If session creation fails, stop the driver service to prevent zombie processes - if (executor instanceof DriverCommandExecutor) { - try { - ((DriverCommandExecutor) executor).close(); - } catch (Exception ignored) { - // Ignore cleanup exceptions, we'll propagate the original failure - } + // If session creation fails, stop the driver service to prevent zombie processes or zombie + // http clients + try (var hce = + executor instanceof HttpCommandExecutor ? (HttpCommandExecutor) executor : null) { + throw e; } - throw e; } } @@ -556,7 +552,8 @@ public void quit() { return; } - try { + try (HttpCommandExecutor httpCommandExecutor = + (executor instanceof HttpCommandExecutor) ? (HttpCommandExecutor) executor : null) { if (this instanceof HasDevTools) { ((HasDevTools) this).maybeGetDevTools().ifPresent(DevTools::close); } diff --git a/java/src/org/openqa/selenium/remote/service/DriverCommandExecutor.java b/java/src/org/openqa/selenium/remote/service/DriverCommandExecutor.java index 177e89d2f076e..131a3c6a4112d 100644 --- a/java/src/org/openqa/selenium/remote/service/DriverCommandExecutor.java +++ b/java/src/org/openqa/selenium/remote/service/DriverCommandExecutor.java @@ -219,6 +219,14 @@ Response invokeExecute(Command command) throws IOException { @Override public void close() { - shutdownGracefully(NAME, executorService); + try (var closeSuper = + new Closeable() { + @Override + public void close() { + DriverCommandExecutor.super.close(); + } + }) { + shutdownGracefully(NAME, executorService); + } } } From 5cd71aa6985a4b5b4d8d670b5d99ecbe297a41b1 Mon Sep 17 00:00:00 2001 From: Diego Molina Date: Thu, 1 Oct 2026 16:28:44 +0200 Subject: [PATCH 2/3] [java] Close traced executors and add tests for HTTP client cleanup Tracing is enabled by default, so RemoteWebDriver usually wraps the HttpCommandExecutor in a TracedCommandExecutor, which the previous HttpCommandExecutor type check skipped. TracedCommandExecutor now forwards close() to its delegate, and RemoteWebDriver closes any Closeable executor when session start fails or on quit(), without letting a close failure mask the original outcome. Also replaces the try-with-resources clean-up idioms with plain try/finally and adds unit tests for plain, traced and quit paths. Co-Authored-By: Claude Opus 5.5 --- .../selenium/remote/RemoteWebDriver.java | 27 +++++--- .../remote/TracedCommandExecutor.java | 10 ++- .../remote/service/DriverCommandExecutor.java | 10 +-- .../RemoteWebDriverInitializationTest.java | 62 +++++++++++++++++++ 4 files changed, 93 insertions(+), 16 deletions(-) diff --git a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java index e5c6ceef11432..c059c722866d7 100644 --- a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java +++ b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java @@ -25,6 +25,7 @@ import com.google.common.net.MediaType; import java.io.BufferedInputStream; +import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.net.MalformedURLException; @@ -319,12 +320,10 @@ protected void startSession(Capabilities capabilities) { sessionId = new SessionId(response.getSessionId()); this.biDi = createBiDi(); } catch (Exception e) { - // If session creation fails, stop the driver service to prevent zombie processes or zombie - // http clients - try (var hce = - executor instanceof HttpCommandExecutor ? (HttpCommandExecutor) executor : null) { - throw e; - } + // quit() is a no-op without a session id, so this is the only chance to release the + // executor's HTTP client and threads before the caller retries with a new driver + closeExecutor(); + throw e; } } @@ -552,8 +551,7 @@ public void quit() { return; } - try (HttpCommandExecutor httpCommandExecutor = - (executor instanceof HttpCommandExecutor) ? (HttpCommandExecutor) executor : null) { + try { if (this instanceof HasDevTools) { ((HasDevTools) this).maybeGetDevTools().ifPresent(DevTools::close); } @@ -563,6 +561,19 @@ public void quit() { execute(DriverCommand.QUIT); } finally { sessionId = null; + closeExecutor(); + } + } + + private void closeExecutor() { + if (!(executor instanceof Closeable)) { + return; + } + try { + ((Closeable) executor).close(); + } catch (Exception e) { + // Never mask the failure (or success) of the operation that triggered the clean-up + LOG.log(Level.FINE, "Failed to close the command executor", e); } } diff --git a/java/src/org/openqa/selenium/remote/TracedCommandExecutor.java b/java/src/org/openqa/selenium/remote/TracedCommandExecutor.java index 89a51edfd2cbb..3f8aa7f42d68c 100644 --- a/java/src/org/openqa/selenium/remote/TracedCommandExecutor.java +++ b/java/src/org/openqa/selenium/remote/TracedCommandExecutor.java @@ -17,13 +17,14 @@ package org.openqa.selenium.remote; +import java.io.Closeable; import java.io.IOException; import java.util.Map; import java.util.Objects; import org.openqa.selenium.remote.tracing.Span; import org.openqa.selenium.remote.tracing.Tracer; -public class TracedCommandExecutor implements CommandExecutor { +public class TracedCommandExecutor implements CommandExecutor, Closeable { private final CommandExecutor delegate; private final Tracer tracer; @@ -51,4 +52,11 @@ public Response execute(Command command) throws IOException { return delegate.execute(command); } } + + @Override + public void close() throws IOException { + if (delegate instanceof Closeable) { + ((Closeable) delegate).close(); + } + } } diff --git a/java/src/org/openqa/selenium/remote/service/DriverCommandExecutor.java b/java/src/org/openqa/selenium/remote/service/DriverCommandExecutor.java index 131a3c6a4112d..2326df0beeaf7 100644 --- a/java/src/org/openqa/selenium/remote/service/DriverCommandExecutor.java +++ b/java/src/org/openqa/selenium/remote/service/DriverCommandExecutor.java @@ -219,14 +219,10 @@ Response invokeExecute(Command command) throws IOException { @Override public void close() { - try (var closeSuper = - new Closeable() { - @Override - public void close() { - DriverCommandExecutor.super.close(); - } - }) { + try { shutdownGracefully(NAME, executorService); + } finally { + super.close(); } } } diff --git a/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java b/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java index 5759b4f253ea8..f23cdd60c2886 100644 --- a/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java +++ b/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java @@ -36,6 +36,7 @@ import java.io.IOException; import java.io.UncheckedIOException; +import java.net.ConnectException; import java.net.MalformedURLException; import java.net.URL; import java.time.Duration; @@ -49,12 +50,14 @@ import org.openqa.selenium.ImmutableCapabilities; import org.openqa.selenium.Platform; import org.openqa.selenium.SessionNotCreatedException; +import org.openqa.selenium.WebDriverException; import org.openqa.selenium.remote.http.ClientConfig; import org.openqa.selenium.remote.http.Contents; import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.http.HttpMethod; import org.openqa.selenium.remote.http.HttpResponse; import org.openqa.selenium.remote.service.DriverCommandExecutor; +import org.openqa.selenium.remote.tracing.empty.NullTracer; @Tag("UnitTests") class RemoteWebDriverInitializationTest { @@ -116,6 +119,65 @@ void closesDriverCommandExecutorWhenSessionCreationFailsAfterNewSessionResponse( verify(executor).close(); } + @Test + void closesHttpClientWhenSessionCreationFails() throws MalformedURLException { + HttpClient client = unreachableRemoteEnd(); + CommandExecutor executor = new HttpCommandExecutor(client, new URL("http://localhost:4444/")); + + assertThatExceptionOfType(WebDriverException.class) + .isThrownBy(() -> new RemoteWebDriver(executor, new ImmutableCapabilities())); + + verify(client).close(); + } + + @Test + void closesHttpClientOfTracedExecutorWhenSessionCreationFails() throws MalformedURLException { + HttpClient client = unreachableRemoteEnd(); + CommandExecutor executor = + new TracedCommandExecutor( + new HttpCommandExecutor(client, new URL("http://localhost:4444/")), new NullTracer()); + + assertThatExceptionOfType(WebDriverException.class) + .isThrownBy(() -> new RemoteWebDriver(executor, new ImmutableCapabilities())); + + verify(client).close(); + } + + @Test + void quitClosesHttpClientEvenIfQuitCommandFails() throws MalformedURLException { + HttpClient client = mock(HttpClient.class); + when(client.execute(any())) + .thenReturn(newSessionResponse()) + .thenThrow(new UncheckedIOException(new ConnectException("Connection refused"))); + RemoteWebDriver driver = + new RemoteWebDriver( + new HttpCommandExecutor(client, new URL("http://localhost:4444/")), + new ImmutableCapabilities()); + + assertThatExceptionOfType(WebDriverException.class).isThrownBy(driver::quit); + + verify(client).close(); + } + + private static HttpClient unreachableRemoteEnd() { + HttpClient client = mock(HttpClient.class); + when(client.execute(any())) + .thenThrow(new UncheckedIOException(new ConnectException("Connection refused"))); + return client; + } + + private static HttpResponse newSessionResponse() { + return new HttpResponse() + .setStatus(200) + .setContent( + Contents.asJson( + singletonMap( + "value", + Map.of( + "sessionId", UUID.randomUUID().toString(), + "capabilities", new ImmutableCapabilities().asMap())))); + } + @Test void constructorShouldThrowIfExecutorReturnsAResponseWithNullValueOnAnAttemptToStartASession() { CommandExecutor executor = WebDriverFixture.prepareExecutorMock(nullValueResponder); From e61093272ea26f646bd451f626d22038f4f520f4 Mon Sep 17 00:00:00 2001 From: Diego Molina Date: Thu, 1 Oct 2026 16:46:55 +0200 Subject: [PATCH 3/3] [java] Let quit() end the remote session before closing the executor If startSession() fails after the session id is set (e.g. while setting up BiDi), the constructor calls quit() to end the remote session. Closing the executor in startSession() first made that QUIT fail, leaving the remote session open. Only close it there when there is no session id; otherwise quit() sends QUIT and then closes the executor. Co-Authored-By: Claude Opus 5.5 --- .../selenium/remote/RemoteWebDriver.java | 9 ++-- .../RemoteWebDriverInitializationTest.java | 45 ++++++++++++++++++- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java index c059c722866d7..8a8732f86592a 100644 --- a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java +++ b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java @@ -320,9 +320,12 @@ protected void startSession(Capabilities capabilities) { sessionId = new SessionId(response.getSessionId()); this.biDi = createBiDi(); } catch (Exception e) { - // quit() is a no-op without a session id, so this is the only chance to release the - // executor's HTTP client and threads before the caller retries with a new driver - closeExecutor(); + // Without a session id quit() is a no-op, so this is the only chance to release the + // executor's HTTP client and threads; with one, the constructor's quit() must still be able + // to end the remote session before it closes the executor + if (sessionId == null) { + closeExecutor(); + } throw e; } } diff --git a/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java b/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java index f23cdd60c2886..2286b36c4dc08 100644 --- a/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java +++ b/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java @@ -24,6 +24,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -40,6 +41,8 @@ import java.net.MalformedURLException; import java.net.URL; import java.time.Duration; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.UUID; import org.jspecify.annotations.NullMarked; @@ -55,6 +58,7 @@ import org.openqa.selenium.remote.http.Contents; import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.http.HttpMethod; +import org.openqa.selenium.remote.http.HttpRequest; import org.openqa.selenium.remote.http.HttpResponse; import org.openqa.selenium.remote.service.DriverCommandExecutor; import org.openqa.selenium.remote.tracing.empty.NullTracer; @@ -159,6 +163,40 @@ void quitClosesHttpClientEvenIfQuitCommandFails() throws MalformedURLException { verify(client).close(); } + @Test + void endsRemoteSessionBeforeClosingHttpClientWhenSetupFailsAfterSessionStarted() + throws MalformedURLException { + List events = new ArrayList<>(); + HttpClient client = mock(HttpClient.class); + when(client.execute(any())) + .thenAnswer( + invocation -> { + HttpRequest request = invocation.getArgument(0); + events.add(request.getMethod().name()); + return request.getMethod() == HttpMethod.POST + ? newSessionResponse(Map.of("webSocketUrl", "ws://localhost:4444/session/bidi")) + : new HttpResponse() + .setStatus(200) + .setContent(Contents.asJson(singletonMap("value", null))); + }); + doAnswer(invocation -> events.add("close")).when(client).close(); + HttpClient.Factory webSocketClientFactory = mock(HttpClient.Factory.class); + when(webSocketClientFactory.createClient(any(ClientConfig.class))) + .thenThrow(new IllegalStateException("Stub WebSocket client failure")); + + assertThatExceptionOfType(IllegalStateException.class) + .isThrownBy( + () -> + new RemoteWebDriver( + new HttpCommandExecutor(client, new URL("http://localhost:4444/")), + new ImmutableCapabilities(), + webSocketClientFactory, + ClientConfig.defaultConfig())) + .withMessage("Stub WebSocket client failure"); + + assertThat(events).startsWith("POST", "DELETE", "close"); + } + private static HttpClient unreachableRemoteEnd() { HttpClient client = mock(HttpClient.class); when(client.execute(any())) @@ -167,6 +205,10 @@ private static HttpClient unreachableRemoteEnd() { } private static HttpResponse newSessionResponse() { + return newSessionResponse(Map.of()); + } + + private static HttpResponse newSessionResponse(Map capabilities) { return new HttpResponse() .setStatus(200) .setContent( @@ -174,8 +216,7 @@ private static HttpResponse newSessionResponse() { singletonMap( "value", Map.of( - "sessionId", UUID.randomUUID().toString(), - "capabilities", new ImmutableCapabilities().asMap())))); + "sessionId", UUID.randomUUID().toString(), "capabilities", capabilities)))); } @Test