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

[dotnet] Propagate service asynchronicity to the command executor. #15246

Open
wants to merge 23 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8fc36d2
[dotnet] Make `DriverService.Start()` thread-safe
RenderMichael Feb 6, 2025
16024d7
Merge branch 'trunk' into concurrent-driver-service
RenderMichael Feb 6, 2025
046dd0e
Simplify `DriverService.IsInitialized`
RenderMichael Feb 6, 2025
aabd21c
Merge branch 'concurrent-driver-service' of https://github.com/Render…
RenderMichael Feb 6, 2025
5d46690
Merge branch 'trunk' into concurrent-driver-service
RenderMichael Feb 6, 2025
6769ec7
Remove thread-safety, implement asynchronicity
RenderMichael Feb 6, 2025
3461067
Merge branch 'concurrent-driver-service' of https://github.com/Render…
RenderMichael Feb 6, 2025
83013be
minimize unnecessary diffs
RenderMichael Feb 6, 2025
60b87b1
minimize diffs
RenderMichael Feb 6, 2025
71b0ab9
Further minimize diffs
RenderMichael Feb 6, 2025
714ca57
Fill in comments
RenderMichael Feb 6, 2025
9b20295
Poll for accessible driver service, even if process has begun
RenderMichael Feb 7, 2025
77b730e
Assign `driverServiceProcess` before polling for process running
RenderMichael Feb 7, 2025
23a7c47
Re-introduce thread-safety
RenderMichael Feb 7, 2025
97aab60
minimize diff
RenderMichael Feb 7, 2025
3df26d2
Wait for service initialization, even if process exists
RenderMichael Feb 7, 2025
8306d87
Simplify thread safety now that we have a wait in place
RenderMichael Feb 7, 2025
d4b91ff
remove unused field
RenderMichael Feb 7, 2025
9fe018d
Fix obsoletion messages
RenderMichael Feb 7, 2025
3d085be
minimize diffs
RenderMichael Feb 7, 2025
589aa45
Merge branch 'trunk' into concurrent-driver-service
RenderMichael Feb 9, 2025
bcb532f
Remove pubic-facing changes
RenderMichael Feb 9, 2025
f423837
Null out disposed value on throw
RenderMichael Feb 9, 2025
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
144 changes: 94 additions & 50 deletions dotnet/src/webdriver/DriverService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -176,33 +176,41 @@ protected virtual bool IsInitialized
{
get
{
bool isInitialized = false;
return Task.Run(this.IsInitializedAsync).GetAwaiter().GetResult();
}
}

try
/// <summary>
/// Gets a value indicating whether the service is responding to HTTP requests.
/// </summary>
/// <returns>A task that represents the asynchronous initialization check operation.</returns>
protected async virtual Task<bool> IsInitializedAsync()
{
bool isInitialized = false;
try
{
using (var httpClient = new HttpClient())
{
using (var httpClient = new HttpClient())
{
httpClient.DefaultRequestHeaders.ConnectionClose = true;
httpClient.Timeout = TimeSpan.FromSeconds(5);
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 = 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.
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)
{
// Do nothing. The exception is expected, meaning driver service is not initialized.
}

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 isInitialized;
}

/// <summary>
Expand All @@ -217,39 +225,55 @@ public void Dispose()
/// <summary>
/// Starts the DriverService if it is not already running.
/// </summary>
[MemberNotNull(nameof(driverServiceProcess))]
public void Start()
{
if (this.driverServiceProcess != null)
{
return;
}

this.driverServiceProcess = new Process();
Task.Run(this.StartAsync).GetAwaiter().GetResult();
}

if (this.DriverServicePath != null)
/// <summary>
/// Starts the DriverService if it is not already running.
/// </summary>
/// <returns>A task that represents the asynchronous start operation.</returns>
public async Task StartAsync()
{
if (this.driverServiceProcess == null)
{
if (this.DriverServiceExecutableName is null)
this.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");
}

this.driverServiceProcess.StartInfo.FileName = Path.Combine(this.DriverServicePath, this.DriverServiceExecutableName);
}
else
{
this.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();
}

this.driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments;
this.driverServiceProcess.StartInfo.UseShellExecute = false;
this.driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow;
this.driverServiceProcess.StartInfo.Arguments = this.CommandLineArguments;
this.driverServiceProcess.StartInfo.UseShellExecute = false;
this.driverServiceProcess.StartInfo.CreateNoWindow = this.HideCommandPromptWindow;

DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(this.driverServiceProcess.StartInfo);
this.OnDriverProcessStarting(eventArgs);
DriverProcessStartingEventArgs eventArgs = new DriverProcessStartingEventArgs(this.driverServiceProcess.StartInfo);
this.OnDriverProcessStarting(eventArgs);

this.driverServiceProcess.Start();
bool serviceAvailable = this.WaitForServiceInitialization();
this.driverServiceProcess.Start();
}
catch
{
this.driverServiceProcess.Dispose();
this.driverServiceProcess = null;
throw;
}
}

bool serviceAvailable = await this.WaitForServiceInitializationAsync().ConfigureAwait(false);
DriverProcessStartedEventArgs processStartedEventArgs = new DriverProcessStartedEventArgs(this.driverServiceProcess);
this.OnDriverProcessStarted(processStartedEventArgs);

Expand All @@ -275,13 +299,33 @@ protected virtual void Dispose(bool disposing)
{
if (disposing)
{
this.Stop();
Task.Run(this.StopAsync).GetAwaiter().GetResult();
}

this.isDisposed = true;
}
}

/// <summary>
/// Releases all resources associated with this <see cref="DriverService"/>.
/// </summary>
/// <returns>A task that represents the asynchronous dispose operation.</returns>
public async ValueTask DisposeAsync()
{
await DisposeAsyncCore();
Dispose(false);
GC.SuppressFinalize(this);
}

/// <summary>
/// Releases all resources associated with this type in the instance's type chain. Override to dispose more resources.
/// </summary>
/// <returns>A task that represents the asynchronous dispose operation.</returns>
protected virtual async ValueTask DisposeAsyncCore()
{
await this.StopAsync().ConfigureAwait(false);
}

/// <summary>
/// Raises the <see cref="DriverProcessStarting"/> event.
/// </summary>
Expand Down Expand Up @@ -313,7 +357,7 @@ protected void OnDriverProcessStarted(DriverProcessStartedEventArgs eventArgs)
/// <summary>
/// Stops the DriverService.
/// </summary>
private void Stop()
private async Task StopAsync()
{
if (this.IsRunning)
{
Expand All @@ -334,7 +378,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))
{

}
Expand Down Expand Up @@ -371,7 +415,7 @@ private void Stop()
/// </summary>
/// <returns><see langword="true"/> if the service is properly started and receiving HTTP requests;
/// otherwise; <see langword="false"/>.</returns>
private bool WaitForServiceInitialization()
private async Task<bool> WaitForServiceInitializationAsync()
{
bool isInitialized = false;
DateTime timeout = DateTime.Now.Add(this.InitializationTimeout);
Expand All @@ -383,7 +427,7 @@ private bool WaitForServiceInitialization()
break;
}

isInitialized = this.IsInitialized;
isInitialized = await this.IsInitializedAsync().ConfigureAwait(false);
}

return isInitialized;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public async Task<Response> 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
Expand Down
Loading