From 8fc36d2d9d7495504d445fc67408acb129b70891 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 6 Feb 2025 00:39:49 -0500 Subject: [PATCH 01/18] [dotnet] Make `DriverService.Start()` thread-safe --- dotnet/src/webdriver/DriverService.cs | 70 ++++++++++++++++----------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index a8907cf976769..e1c142f170543 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -37,6 +37,7 @@ namespace OpenQA.Selenium public abstract class DriverService : ICommandServer { private bool isDisposed; + private readonly object driverServiceProcessLock = new(); private Process? driverServiceProcess; /// @@ -220,42 +221,55 @@ public void Dispose() [MemberNotNull(nameof(driverServiceProcess))] public void Start() { - if (this.driverServiceProcess != null) + if (this.driverServiceProcess is null) { - return; - } + lock (this.driverServiceProcessLock) + { + if (this.driverServiceProcess is null) + { + var driverServiceProcess = new Process(); - this.driverServiceProcess = new Process(); + try + { + if (this.DriverServicePath != null) + { + if (this.DriverServiceExecutableName is null) + { + throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); + } - if (this.DriverServicePath != null) - { - if (this.DriverServiceExecutableName is null) - { - throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); - } + driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); + } + else + { + driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); + } - this.driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); - } - else - { - this.driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); - } + driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; + driverServiceProcess.StartInfo.UseShellExecute = false; + driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; - this.driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; - this.driverServiceProcess.StartInfo.UseShellExecute = false; - this.driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; + this.OnDriverProcessStarting(new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo)); - DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(this.driverServiceProcess.StartInfo); - this.OnDriverProcessStarting(eventArgs); + driverServiceProcess.Start(); + bool serviceAvailable = this.WaitForServiceInitialization(); - this.driverServiceProcess.Start(); - bool serviceAvailable = this.WaitForServiceInitialization(); - DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(this.driverServiceProcess); - this.OnDriverProcessStarted(processStartedEventArgs); + this.OnDriverProcessStarted(new DriverProcessStartedEventArgs(driverServiceProcess)); - if (!serviceAvailable) - { - throw new WebDriverException($"Cannot start the driver service on {this.ServiceUrl}"); + if (!serviceAvailable) + { + throw new WebDriverException($"Cannot start the driver service on {this.ServiceUrl}"); + } + } + catch + { + driverServiceProcess.Dispose(); + throw; + } + + this.driverServiceProcess = driverServiceProcess; + } + } } } From 046dd0edf498fc0bd4f30a10dac6263d4222dd24 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 6 Feb 2025 01:18:49 -0500 Subject: [PATCH 02/18] Simplify `DriverService.IsInitialized` --- dotnet/src/webdriver/DriverService.cs | 33 ++++++++++++--------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index e1c142f170543..68ff4ab25a540 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -177,32 +177,27 @@ protected virtual bool IsInitialized { get { - bool isInitialized = false; - try { - using (var httpClient = new HttpClient()) - { - httpClient.DefaultRequestHeaders.ConnectionClose = true; - httpClient.Timeout = TimeSpan.FromSeconds(5); + using var httpClient = new HttpClient(); + httpClient.DefaultRequestHeaders.ConnectionClose = true; + httpClient.Timeout = TimeSpan.FromSeconds(5); - Uri serviceHealthUri = new Uri(this.ServiceUrl, new Uri(DriverCommand.Status, UriKind.Relative)); - using (var response = Task.Run(async () => await httpClient.GetAsync(serviceHealthUri)).GetAwaiter().GetResult()) - { - // Checking the response from the 'status' end point. Note that we are simply checking - // that the HTTP status returned is a 200 status, and that the resposne has the correct - // Content-Type header. A more sophisticated check would parse the JSON response and - // validate its values. At the moment we do not do this more sophisticated check. - isInitialized = response.StatusCode == HttpStatusCode.OK && response.Content.Headers.ContentType is { MediaType: string mediaType } && mediaType.StartsWith("application/json", StringComparison.OrdinalIgnoreCase); - } - } + Uri serviceHealthUri = new Uri(this.ServiceUrl, new Uri(DriverCommand.Status, UriKind.Relative)); + using var response = Task.Run(async () => await httpClient.GetAsync(serviceHealthUri)).GetAwaiter().GetResult(); + + // Checking the response from the 'status' end point. Note that we are simply checking + // that the HTTP status returned is a 200 status, and that the response has the correct + // Content-Type header. A more sophisticated check would parse the JSON response and + // validate its values. At the moment we do not do this more sophisticated check. + bool isInitialized = response.StatusCode == HttpStatusCode.OK && response.Content.Headers.ContentType is { MediaType: string mediaType } && mediaType.StartsWith("application/json", StringComparison.OrdinalIgnoreCase); + + return isInitialized; } catch (Exception ex) when (ex is HttpRequestException || ex is TaskCanceledException) { - // Do nothing. The exception is expected, meaning driver service is not initialized. + return false; } - - return isInitialized; } } From 6769ec7148a89471e28d7f5a9e939b60b7d2466b Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 6 Feb 2025 16:47:14 -0500 Subject: [PATCH 03/18] Remove thread-safety, implement asynchronicity --- dotnet/src/webdriver/DriverService.cs | 154 +++++++++++------- .../Remote/DriverServiceCommandExecutor.cs | 2 +- dotnet/src/webdriver/Remote/ICommandServer.cs | 10 +- 3 files changed, 103 insertions(+), 63 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index 68ff4ab25a540..6c38dcb2ca9e3 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -37,7 +37,6 @@ namespace OpenQA.Selenium public abstract class DriverService : ICommandServer { private bool isDisposed; - private readonly object driverServiceProcessLock = new(); private Process? driverServiceProcess; /// @@ -173,31 +172,41 @@ public int ProcessId /// /// Gets a value indicating whether the service is responding to HTTP requests. /// + [Obsolete("Use the asynchronous overload IsInitializedAsync. The synchronous version will be removed in Selenium 4.31")] protected virtual bool IsInitialized { get { - try - { - using var httpClient = new HttpClient(); - httpClient.DefaultRequestHeaders.ConnectionClose = true; - httpClient.Timeout = TimeSpan.FromSeconds(5); + return Task.Run(async () => await IsInitializedAsync()).GetAwaiter().GetResult(); + } + } - Uri serviceHealthUri = new Uri(this.ServiceUrl, new Uri(DriverCommand.Status, UriKind.Relative)); - using var response = Task.Run(async () => await httpClient.GetAsync(serviceHealthUri)).GetAwaiter().GetResult(); + /// + /// Gets a value indicating whether the service is responding to HTTP requests. + /// + /// A task that represents the asynchronous initialization check operation. + protected async virtual Task IsInitializedAsync() + { + try + { + using var httpClient = new HttpClient(); + httpClient.DefaultRequestHeaders.ConnectionClose = true; + httpClient.Timeout = TimeSpan.FromSeconds(5); - // Checking the response from the 'status' end point. Note that we are simply checking - // that the HTTP status returned is a 200 status, and that the response has the correct - // Content-Type header. A more sophisticated check would parse the JSON response and - // validate its values. At the moment we do not do this more sophisticated check. - bool isInitialized = response.StatusCode == HttpStatusCode.OK && response.Content.Headers.ContentType is { MediaType: string mediaType } && mediaType.StartsWith("application/json", StringComparison.OrdinalIgnoreCase); + Uri serviceHealthUri = new Uri(this.ServiceUrl, new Uri(DriverCommand.Status, UriKind.Relative)); + using var response = await httpClient.GetAsync(serviceHealthUri); - return isInitialized; - } - catch (Exception ex) when (ex is HttpRequestException || ex is TaskCanceledException) - { - return false; - } + // Checking the response from the 'status' end point. Note that we are simply checking + // that the HTTP status returned is a 200 status, and that the response has the correct + // Content-Type header. A more sophisticated check would parse the JSON response and + // validate its values. At the moment we do not do this more sophisticated check. + bool isInitialized = response.StatusCode == HttpStatusCode.OK && response.Content.Headers.ContentType is { MediaType: string mediaType } && mediaType.StartsWith("application/json", StringComparison.OrdinalIgnoreCase); + + return isInitialized; + } + catch (Exception ex) when (ex is HttpRequestException || ex is TaskCanceledException) + { + return false; } } @@ -213,58 +222,61 @@ public void Dispose() /// /// Starts the DriverService if it is not already running. /// - [MemberNotNull(nameof(driverServiceProcess))] + [Obsolete("Use the asynchronous overload IsInitializedAsync. The synchronous version will be removed in Selenium 4.31")] public void Start() + { + Task.Run(async () => await StartAsync()).GetAwaiter().GetResult(); + } + + /// + /// Starts the DriverService if it is not already running. + /// + /// A task that represents the asynchronous start operation. + public async Task StartAsync() { if (this.driverServiceProcess is null) { - lock (this.driverServiceProcessLock) + var driverServiceProcess = new Process(); + + try { - if (this.driverServiceProcess is null) + if (this.DriverServicePath != null) { - var driverServiceProcess = new Process(); - - try + if (this.DriverServiceExecutableName is null) { - if (this.DriverServicePath != null) - { - if (this.DriverServiceExecutableName is null) - { - throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); - } - - driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); - } - else - { - driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); - } + throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); + } - driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; - driverServiceProcess.StartInfo.UseShellExecute = false; - driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; + driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); + } + else + { + driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); + } - this.OnDriverProcessStarting(new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo)); + driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; + driverServiceProcess.StartInfo.UseShellExecute = false; + driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; - driverServiceProcess.Start(); - bool serviceAvailable = this.WaitForServiceInitialization(); + this.OnDriverProcessStarting(new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo)); - this.OnDriverProcessStarted(new DriverProcessStartedEventArgs(driverServiceProcess)); + driverServiceProcess.Start(); + bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false); - if (!serviceAvailable) - { - throw new WebDriverException($"Cannot start the driver service on {this.ServiceUrl}"); - } - } - catch - { - driverServiceProcess.Dispose(); - throw; - } + this.OnDriverProcessStarted(new DriverProcessStartedEventArgs(driverServiceProcess)); - this.driverServiceProcess = driverServiceProcess; + if (!serviceAvailable) + { + throw new WebDriverException($"Cannot start the driver service on {this.ServiceUrl}"); } } + catch + { + driverServiceProcess.Dispose(); + throw; + } + + this.driverServiceProcess = driverServiceProcess; } } @@ -284,13 +296,33 @@ protected virtual void Dispose(bool disposing) { if (disposing) { - this.Stop(); + Task.Run(() => this.StopAsync()).GetAwaiter().GetResult(); } this.isDisposed = true; } } + /// + /// Releases all resources associated with this . + /// + /// A task that represents the asynchronous dispose operation. + public async ValueTask DisposeAsync() + { + await DisposeAsyncCore(); + Dispose(false); + GC.SuppressFinalize(this); + } + + /// + /// + /// + /// + protected virtual async ValueTask DisposeAsyncCore() + { + await this.StopAsync().ConfigureAwait(false); + } + /// /// Raises the event. /// @@ -322,7 +354,7 @@ protected void OnDriverProcessStarted(DriverProcessStartedEventArgs eventArgs) /// /// Stops the DriverService. /// - private void Stop() + private async Task StopAsync() { if (this.IsRunning) { @@ -343,7 +375,7 @@ private void Stop() // we'll retry. We wait for exit here, since catching the exception // for a failed HTTP request due to a closed socket is particularly // expensive. - using (var response = Task.Run(async () => await httpClient.GetAsync(shutdownUrl)).GetAwaiter().GetResult()) + using (var response = await httpClient.GetAsync(shutdownUrl).ConfigureAwait(false)) { } @@ -380,7 +412,7 @@ private void Stop() /// /// if the service is properly started and receiving HTTP requests; /// otherwise; . - private bool WaitForServiceInitialization() + private async Task WaitForServiceInitializationAsync() { bool isInitialized = false; DateTime timeout = DateTime.Now.Add(this.InitializationTimeout); @@ -392,7 +424,7 @@ private bool WaitForServiceInitialization() break; } - isInitialized = this.IsInitialized; + isInitialized = await this.IsInitializedAsync().ConfigureAwait(false); } return isInitialized; diff --git a/dotnet/src/webdriver/Remote/DriverServiceCommandExecutor.cs b/dotnet/src/webdriver/Remote/DriverServiceCommandExecutor.cs index 0f1d341c8c353..3f17f809cae2a 100644 --- a/dotnet/src/webdriver/Remote/DriverServiceCommandExecutor.cs +++ b/dotnet/src/webdriver/Remote/DriverServiceCommandExecutor.cs @@ -117,7 +117,7 @@ public async Task ExecuteAsync(Command commandToExecute) Response toReturn; if (commandToExecute.Name == DriverCommand.NewSession) { - this.service.Start(); + await this.service.StartAsync().ConfigureAwait(false); } // Use a try-catch block to catch exceptions for the Quit diff --git a/dotnet/src/webdriver/Remote/ICommandServer.cs b/dotnet/src/webdriver/Remote/ICommandServer.cs index f70d536dfd042..9dadec20036ae 100644 --- a/dotnet/src/webdriver/Remote/ICommandServer.cs +++ b/dotnet/src/webdriver/Remote/ICommandServer.cs @@ -18,6 +18,7 @@ // using System; +using System.Threading.Tasks; #nullable enable @@ -26,11 +27,18 @@ namespace OpenQA.Selenium.Remote /// /// Provides a way to start a server that understands remote commands /// - public interface ICommandServer : IDisposable + public interface ICommandServer : IDisposable, IAsyncDisposable { /// /// Starts the server. /// + [Obsolete("Use the asynchronous overload IsInitializedAsync. The synchronous version will be removed in Selenium 4.31")] void Start(); + + /// + /// Starts the server. + /// + /// A task that represents the asynchronous start operation. + Task StartAsync(); } } From 83013bee428cadff8b2233975c37afe0347cd613 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 6 Feb 2025 16:50:13 -0500 Subject: [PATCH 04/18] minimize unnecessary diffs --- dotnet/src/webdriver/DriverService.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index 6c38dcb2ca9e3..a1174d3bd9434 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -258,12 +258,14 @@ public async Task StartAsync() driverServiceProcess.StartInfo.UseShellExecute = false; driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; - this.OnDriverProcessStarting(new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo)); + DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo); + this.OnDriverProcessStarting(eventArgs); driverServiceProcess.Start(); bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false); - this.OnDriverProcessStarted(new DriverProcessStartedEventArgs(driverServiceProcess)); + DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(driverServiceProcess); + this.OnDriverProcessStarted(processStartedEventArgs); if (!serviceAvailable) { From 60b87b10989c81b2153c70771a848107080ccc0a Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 6 Feb 2025 16:51:43 -0500 Subject: [PATCH 05/18] minimize diffs --- dotnet/src/webdriver/DriverService.cs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index a1174d3bd9434..0b68655323303 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -189,20 +189,23 @@ protected async virtual Task IsInitializedAsync() { try { - using var httpClient = new HttpClient(); - httpClient.DefaultRequestHeaders.ConnectionClose = true; - httpClient.Timeout = TimeSpan.FromSeconds(5); - - Uri serviceHealthUri = new Uri(this.ServiceUrl, new Uri(DriverCommand.Status, UriKind.Relative)); - using var response = await httpClient.GetAsync(serviceHealthUri); + using (var httpClient = new HttpClient()) + { + httpClient.DefaultRequestHeaders.ConnectionClose = true; + httpClient.Timeout = TimeSpan.FromSeconds(5); - // Checking the response from the 'status' end point. Note that we are simply checking - // that the HTTP status returned is a 200 status, and that the response has the correct - // Content-Type header. A more sophisticated check would parse the JSON response and - // validate its values. At the moment we do not do this more sophisticated check. - bool isInitialized = response.StatusCode == HttpStatusCode.OK && response.Content.Headers.ContentType is { MediaType: string mediaType } && mediaType.StartsWith("application/json", StringComparison.OrdinalIgnoreCase); + Uri serviceHealthUri = new Uri(this.ServiceUrl, new Uri(DriverCommand.Status, UriKind.Relative)); + using (var response = await httpClient.GetAsync(serviceHealthUri)) + { + // Checking the response from the 'status' end point. Note that we are simply checking + // that the HTTP status returned is a 200 status, and that the response has the correct + // Content-Type header. A more sophisticated check would parse the JSON response and + // validate its values. At the moment we do not do this more sophisticated check. + bool isInitialized = response.StatusCode == HttpStatusCode.OK && response.Content.Headers.ContentType is { MediaType: string mediaType } && mediaType.StartsWith("application/json", StringComparison.OrdinalIgnoreCase); - return isInitialized; + return isInitialized; + } + } } catch (Exception ex) when (ex is HttpRequestException || ex is TaskCanceledException) { From 71b0ab92f48fc51975a11e9a30c030b3caa25087 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 6 Feb 2025 16:52:57 -0500 Subject: [PATCH 06/18] Further minimize diffs --- dotnet/src/webdriver/DriverService.cs | 68 ++++++++++++++------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index 0b68655323303..be267477bcc36 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -237,52 +237,54 @@ public void Start() /// A task that represents the asynchronous start operation. public async Task StartAsync() { - if (this.driverServiceProcess is null) + if (this.driverServiceProcess != null) { - var driverServiceProcess = new Process(); + return; + } - try - { - if (this.DriverServicePath != null) - { - if (this.DriverServiceExecutableName is null) - { - throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); - } + var driverServiceProcess = new Process(); - driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); - } - else + try + { + if (this.DriverServicePath != null) + { + if (this.DriverServiceExecutableName is null) { - driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); + throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); } - driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; - driverServiceProcess.StartInfo.UseShellExecute = false; - driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; + driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); + } + else + { + driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); + } + + driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; + driverServiceProcess.StartInfo.UseShellExecute = false; + driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; - DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo); - this.OnDriverProcessStarting(eventArgs); + DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo); + this.OnDriverProcessStarting(eventArgs); - driverServiceProcess.Start(); - bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false); + driverServiceProcess.Start(); + bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false); - DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(driverServiceProcess); - this.OnDriverProcessStarted(processStartedEventArgs); + DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(driverServiceProcess); + this.OnDriverProcessStarted(processStartedEventArgs); - if (!serviceAvailable) - { - throw new WebDriverException($"Cannot start the driver service on {this.ServiceUrl}"); - } - } - catch + if (!serviceAvailable) { - driverServiceProcess.Dispose(); - throw; + throw new WebDriverException($"Cannot start the driver service on {this.ServiceUrl}"); } - - this.driverServiceProcess = driverServiceProcess; } + catch + { + driverServiceProcess.Dispose(); + throw; + } + + this.driverServiceProcess = driverServiceProcess; } /// From 714ca5715dd23500aa7c44eb91681cba5117dc55 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 6 Feb 2025 16:54:48 -0500 Subject: [PATCH 07/18] Fill in comments --- dotnet/src/webdriver/DriverService.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index be267477bcc36..c632ae3b98532 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -269,7 +269,6 @@ public async Task StartAsync() driverServiceProcess.Start(); bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false); - DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(driverServiceProcess); this.OnDriverProcessStarted(processStartedEventArgs); @@ -322,9 +321,9 @@ public async ValueTask DisposeAsync() } /// - /// + /// Releases all resources associated with this type in the instance's type chain. Override to dispose more resources. /// - /// + /// A task that represents the asynchronous dispose operation. protected virtual async ValueTask DisposeAsyncCore() { await this.StopAsync().ConfigureAwait(false); From 9b20295f286b40c55f2ffb6166a0e7f31b3cd621 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 6 Feb 2025 20:43:40 -0500 Subject: [PATCH 08/18] Poll for accessible driver service, even if process has begun --- dotnet/src/webdriver/DriverService.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index c632ae3b98532..f5be00e77c88e 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -239,6 +239,12 @@ public async Task StartAsync() { if (this.driverServiceProcess != null) { + bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false); + if (!serviceAvailable) + { + throw new WebDriverException($"Cannot start the driver service on {this.ServiceUrl}"); + } + return; } From 77b730e3746035eeed8525d611b8f084a6b3cb26 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 6 Feb 2025 20:57:00 -0500 Subject: [PATCH 09/18] Assign `driverServiceProcess` before polling for process running --- dotnet/src/webdriver/DriverService.cs | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index f5be00e77c88e..0d1cd27f96d5a 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -239,12 +239,6 @@ public async Task StartAsync() { if (this.driverServiceProcess != null) { - bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false); - if (!serviceAvailable) - { - throw new WebDriverException($"Cannot start the driver service on {this.ServiceUrl}"); - } - return; } @@ -274,14 +268,6 @@ public async Task StartAsync() this.OnDriverProcessStarting(eventArgs); driverServiceProcess.Start(); - bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false); - DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(driverServiceProcess); - this.OnDriverProcessStarted(processStartedEventArgs); - - if (!serviceAvailable) - { - throw new WebDriverException($"Cannot start the driver service on {this.ServiceUrl}"); - } } catch { @@ -290,6 +276,15 @@ public async Task StartAsync() } this.driverServiceProcess = driverServiceProcess; + + bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false); + DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(driverServiceProcess); + this.OnDriverProcessStarted(processStartedEventArgs); + + if (!serviceAvailable) + { + throw new WebDriverException($"Cannot start the driver service on {this.ServiceUrl}"); + } } /// From 23a7c471f6fc5d17820dfd75d21f30876efa21f2 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 7 Feb 2025 10:35:30 -0500 Subject: [PATCH 10/18] Re-introduce thread-safety --- dotnet/src/webdriver/DriverService.cs | 60 +++++++++++++++------------ 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index 0d1cd27f96d5a..cbb32d54d0127 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -37,6 +37,7 @@ namespace OpenQA.Selenium public abstract class DriverService : ICommandServer { private bool isDisposed; + private readonly object driverServiceProcessLock = new object(); private Process? driverServiceProcess; /// @@ -242,40 +243,47 @@ public async Task StartAsync() return; } - var driverServiceProcess = new Process(); - - try + lock (this.driverServiceProcessLock) { - if (this.DriverServicePath != null) + + if (this.driverServiceProcess == null) { - if (this.DriverServiceExecutableName is null) + var driverServiceProcess = new Process(); + + try { - throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); - } + if (this.DriverServicePath != null) + { + if (this.DriverServiceExecutableName is null) + { + throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); + } - driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); - } - else - { - driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); - } + driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); + } + else + { + driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); + } - driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; - driverServiceProcess.StartInfo.UseShellExecute = false; - driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; + driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; + driverServiceProcess.StartInfo.UseShellExecute = false; + driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; - DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo); - this.OnDriverProcessStarting(eventArgs); + DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo); + this.OnDriverProcessStarting(eventArgs); - driverServiceProcess.Start(); - } - catch - { - driverServiceProcess.Dispose(); - throw; - } + driverServiceProcess.Start(); + } + catch + { + driverServiceProcess.Dispose(); + throw; + } - this.driverServiceProcess = driverServiceProcess; + this.driverServiceProcess = driverServiceProcess; + } + } bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false); DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(driverServiceProcess); From 97aab60b4571819f5358168fa2114e857697dc1d Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 7 Feb 2025 10:37:04 -0500 Subject: [PATCH 11/18] minimize diff --- dotnet/src/webdriver/DriverService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index cbb32d54d0127..a178fb85c51da 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -286,7 +286,7 @@ public async Task StartAsync() } bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false); - DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(driverServiceProcess); + DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(this.driverServiceProcess); this.OnDriverProcessStarted(processStartedEventArgs); if (!serviceAvailable) From 3df26d2a8321e017976762b1c6ccd7997c9850d6 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 7 Feb 2025 10:48:50 -0500 Subject: [PATCH 12/18] Wait for service initialization, even if process exists --- dotnet/src/webdriver/DriverService.cs | 60 +++++++++++++-------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index a178fb85c51da..b10447290e4fa 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -238,50 +238,48 @@ public void Start() /// A task that represents the asynchronous start operation. public async Task StartAsync() { - if (this.driverServiceProcess != null) + if (this.driverServiceProcess == null) { - return; - } - - lock (this.driverServiceProcessLock) - { - - if (this.driverServiceProcess == null) + lock (this.driverServiceProcessLock) { - var driverServiceProcess = new Process(); - try + if (this.driverServiceProcess == null) { - if (this.DriverServicePath != null) + var driverServiceProcess = new Process(); + + try { - if (this.DriverServiceExecutableName is null) + if (this.DriverServicePath != null) + { + if (this.DriverServiceExecutableName is null) + { + throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); + } + + driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); + } + else { - throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); + driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); } - driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); + driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; + driverServiceProcess.StartInfo.UseShellExecute = false; + driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; + + DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo); + this.OnDriverProcessStarting(eventArgs); + + driverServiceProcess.Start(); } - else + catch { - driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); + driverServiceProcess.Dispose(); + throw; } - driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; - driverServiceProcess.StartInfo.UseShellExecute = false; - driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; - - DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo); - this.OnDriverProcessStarting(eventArgs); - - driverServiceProcess.Start(); - } - catch - { - driverServiceProcess.Dispose(); - throw; + this.driverServiceProcess = driverServiceProcess; } - - this.driverServiceProcess = driverServiceProcess; } } From 8306d875b7abb4dc2a296754a403dd827ff4c5fe Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 7 Feb 2025 10:50:11 -0500 Subject: [PATCH 13/18] Simplify thread safety now that we have a wait in place --- dotnet/src/webdriver/DriverService.cs | 57 +++++++++++---------------- 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index b10447290e4fa..50291fd04f3bb 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -240,46 +240,37 @@ public async Task StartAsync() { if (this.driverServiceProcess == null) { - lock (this.driverServiceProcessLock) - { + this.driverServiceProcess = new Process(); - if (this.driverServiceProcess == null) + try + { + if (this.DriverServicePath != null) { - var driverServiceProcess = new Process(); - - try + if (this.DriverServiceExecutableName is null) { - if (this.DriverServicePath != null) - { - if (this.DriverServiceExecutableName is null) - { - throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); - } - - driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); - } - else - { - driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); - } + throw new InvalidOperationException("If the driver service path is specified, the driver service executable name must be as well"); + } - driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; - driverServiceProcess.StartInfo.UseShellExecute = false; - driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; + this.driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName); + } + else + { + this.driverServiceProcess.StartInfo.FileName = new DriverFinder(this.GetDefaultDriverOptions()).GetDriverPath(); + } - DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(driverServiceProcess.StartInfo); - this.OnDriverProcessStarting(eventArgs); + this.driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments; + this.driverServiceProcess.StartInfo.UseShellExecute = false; + this.driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow; - driverServiceProcess.Start(); - } - catch - { - driverServiceProcess.Dispose(); - throw; - } + DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(this.driverServiceProcess.StartInfo); + this.OnDriverProcessStarting(eventArgs); - this.driverServiceProcess = driverServiceProcess; - } + this.driverServiceProcess.Start(); + } + catch + { + this.driverServiceProcess.Dispose(); + throw; } } From d4b91ff4cfc4c8a6444157c59f5e536b01e47d94 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 7 Feb 2025 10:51:06 -0500 Subject: [PATCH 14/18] remove unused field --- dotnet/src/webdriver/DriverService.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index 50291fd04f3bb..172ca41e669e5 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -37,7 +37,6 @@ namespace OpenQA.Selenium public abstract class DriverService : ICommandServer { private bool isDisposed; - private readonly object driverServiceProcessLock = new object(); private Process? driverServiceProcess; /// From 9fe018d324feae0526af1fd7180729956376a719 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 7 Feb 2025 10:52:19 -0500 Subject: [PATCH 15/18] Fix obsoletion messages --- dotnet/src/webdriver/DriverService.cs | 4 ++-- dotnet/src/webdriver/Remote/ICommandServer.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index 172ca41e669e5..c315d52e3a4fe 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -172,7 +172,7 @@ public int ProcessId /// /// Gets a value indicating whether the service is responding to HTTP requests. /// - [Obsolete("Use the asynchronous overload IsInitializedAsync. The synchronous version will be removed in Selenium 4.31")] + [Obsolete("Use the asynchronous method IsInitializedAsync")] protected virtual bool IsInitialized { get @@ -225,7 +225,7 @@ public void Dispose() /// /// Starts the DriverService if it is not already running. /// - [Obsolete("Use the asynchronous overload IsInitializedAsync. The synchronous version will be removed in Selenium 4.31")] + [Obsolete("Use the asynchronous method StartAsync")] public void Start() { Task.Run(async () => await StartAsync()).GetAwaiter().GetResult(); diff --git a/dotnet/src/webdriver/Remote/ICommandServer.cs b/dotnet/src/webdriver/Remote/ICommandServer.cs index 9dadec20036ae..3ffdc9aac56cf 100644 --- a/dotnet/src/webdriver/Remote/ICommandServer.cs +++ b/dotnet/src/webdriver/Remote/ICommandServer.cs @@ -32,7 +32,7 @@ public interface ICommandServer : IDisposable, IAsyncDisposable /// /// Starts the server. /// - [Obsolete("Use the asynchronous overload IsInitializedAsync. The synchronous version will be removed in Selenium 4.31")] + [Obsolete("Use the asynchronous method StartAsync")] void Start(); /// From 3d085be1c5be1cfc093f5cc074a56c8279af0c6d Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 7 Feb 2025 11:36:27 -0500 Subject: [PATCH 16/18] minimize diffs --- dotnet/src/webdriver/DriverService.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index c315d52e3a4fe..7ded8eb1d8736 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -187,6 +187,7 @@ protected virtual bool IsInitialized /// A task that represents the asynchronous initialization check operation. protected async virtual Task IsInitializedAsync() { + bool isInitialized = false; try { using (var httpClient = new HttpClient()) @@ -201,16 +202,16 @@ protected async virtual Task IsInitializedAsync() // that the HTTP status returned is a 200 status, and that the response has the correct // Content-Type header. A more sophisticated check would parse the JSON response and // validate its values. At the moment we do not do this more sophisticated check. - bool isInitialized = response.StatusCode == HttpStatusCode.OK && response.Content.Headers.ContentType is { MediaType: string mediaType } && mediaType.StartsWith("application/json", StringComparison.OrdinalIgnoreCase); - - return isInitialized; + isInitialized = response.StatusCode == HttpStatusCode.OK && response.Content.Headers.ContentType is { MediaType: string mediaType } && mediaType.StartsWith("application/json", StringComparison.OrdinalIgnoreCase); } } } catch (Exception ex) when (ex is HttpRequestException || ex is TaskCanceledException) { - return false; + // Do nothing. The exception is expected, meaning driver service is not initialized. } + + return isInitialized; } /// From bcb532f1a97d1976ba3daa0f781d0d68ebcc7a6a Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sun, 9 Feb 2025 02:22:25 -0500 Subject: [PATCH 17/18] Remove pubic-facing changes --- dotnet/src/webdriver/DriverService.cs | 8 +++----- dotnet/src/webdriver/Remote/ICommandServer.cs | 10 +--------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index 7ded8eb1d8736..dda64ecf338d4 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -172,12 +172,11 @@ public int ProcessId /// /// Gets a value indicating whether the service is responding to HTTP requests. /// - [Obsolete("Use the asynchronous method IsInitializedAsync")] protected virtual bool IsInitialized { get { - return Task.Run(async () => await IsInitializedAsync()).GetAwaiter().GetResult(); + return Task.Run(this.IsInitializedAsync).GetAwaiter().GetResult(); } } @@ -226,10 +225,9 @@ public void Dispose() /// /// Starts the DriverService if it is not already running. /// - [Obsolete("Use the asynchronous method StartAsync")] public void Start() { - Task.Run(async () => await StartAsync()).GetAwaiter().GetResult(); + Task.Run(this.StartAsync).GetAwaiter().GetResult(); } /// @@ -300,7 +298,7 @@ protected virtual void Dispose(bool disposing) { if (disposing) { - Task.Run(() => this.StopAsync()).GetAwaiter().GetResult(); + Task.Run(this.StopAsync).GetAwaiter().GetResult(); } this.isDisposed = true; diff --git a/dotnet/src/webdriver/Remote/ICommandServer.cs b/dotnet/src/webdriver/Remote/ICommandServer.cs index 3ffdc9aac56cf..f70d536dfd042 100644 --- a/dotnet/src/webdriver/Remote/ICommandServer.cs +++ b/dotnet/src/webdriver/Remote/ICommandServer.cs @@ -18,7 +18,6 @@ // using System; -using System.Threading.Tasks; #nullable enable @@ -27,18 +26,11 @@ namespace OpenQA.Selenium.Remote /// /// Provides a way to start a server that understands remote commands /// - public interface ICommandServer : IDisposable, IAsyncDisposable + public interface ICommandServer : IDisposable { /// /// Starts the server. /// - [Obsolete("Use the asynchronous method StartAsync")] void Start(); - - /// - /// Starts the server. - /// - /// A task that represents the asynchronous start operation. - Task StartAsync(); } } From f42383732567a9d59218c9e363bb01fd5b1c3e50 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sun, 9 Feb 2025 02:23:49 -0500 Subject: [PATCH 18/18] Null out disposed value on throw --- dotnet/src/webdriver/DriverService.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/dotnet/src/webdriver/DriverService.cs b/dotnet/src/webdriver/DriverService.cs index dda64ecf338d4..9aee2990b9f2f 100644 --- a/dotnet/src/webdriver/DriverService.cs +++ b/dotnet/src/webdriver/DriverService.cs @@ -268,6 +268,7 @@ public async Task StartAsync() catch { this.driverServiceProcess.Dispose(); + this.driverServiceProcess = null; throw; } }