Skip to content

Commit

Permalink
Remove various timeouts (#27699)
Browse files Browse the repository at this point in the history 
* Remove timeouts from BlazorWebAssemblyDeltaApplier

* Remove timeout from delta applier.

* Remove comment

* Fail when capabilities can't be retrieved
  • Loading branch information
@tmat
tmat authored Sep 14, 2022
1 parent f0b0c15 commit ae921da
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 102 deletions.
1 change: 0 additions & 1 deletion src/BuiltInTools/dotnet-watch/Filters/BrowserRefreshServer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ private async Task WebSocketRequest(HttpContext context)

public async Task WaitForClientConnectionAsync(CancellationToken cancellationToken)
{
_reporter.Verbose("Waiting for a browser to connect");
await _clientConnected.Task.WaitAsync(cancellationToken);
}

Expand Down
73 changes: 23 additions & 50 deletions src/BuiltInTools/dotnet-watch/HotReload/BlazorWebAssemblyDeltaApplier.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,10 @@ namespace Microsoft.DotNet.Watcher.Tools
{
internal class BlazorWebAssemblyDeltaApplier : IDeltaApplier
{
private static Task<ImmutableArray<string>>? _cachedCapabilties;
private static readonly ImmutableArray<string> _baselineCapabilities = ImmutableArray.Create<string>("Baseline");
private static Task<ImmutableArray<string>>? s_cachedCapabilties;
private readonly IReporter _reporter;
private int _sequenceId;

private static readonly TimeSpan VerifyDeltaTimeout = TimeSpan.FromSeconds(5);

public BlazorWebAssemblyDeltaApplier(IReporter reporter)
{
_reporter = reporter;
Expand All @@ -40,57 +37,48 @@ public ValueTask InitializeAsync(DotNetWatchContext context, CancellationToken c

public Task<ImmutableArray<string>> GetApplyUpdateCapabilitiesAsync(DotNetWatchContext context, CancellationToken cancellationToken)
{
_cachedCapabilties ??= GetApplyUpdateCapabilitiesCoreAsync();
return _cachedCapabilties;
return s_cachedCapabilties ??= GetApplyUpdateCapabilitiesCoreAsync();

async Task<ImmutableArray<string>> GetApplyUpdateCapabilitiesCoreAsync()
{
if (context.BrowserRefreshServer is null)
{
return _baselineCapabilities;
throw new ApplicationException("The browser refresh server is unavailable.");
}

await context.BrowserRefreshServer.WaitForClientConnectionAsync(cancellationToken);
_reporter.Verbose("Connecting to the browser.");

await context.BrowserRefreshServer.WaitForClientConnectionAsync(cancellationToken);
await context.BrowserRefreshServer.SendJsonSerlialized(default(BlazorRequestApplyUpdateCapabilities), cancellationToken);
// 32k ought to be enough for anyone.

var buffer = ArrayPool<byte>.Shared.Rent(32 * 1024);
try
{
// We'll query the browser and ask it send capabilities. If the browser does not respond in a short duration, we'll assume something is amiss and return
// baseline capabilities.
var response = await context.BrowserRefreshServer.ReceiveAsync(buffer, cancellationToken)
.AsTask()
.WaitAsync(TimeSpan.FromSeconds(15), cancellationToken);
// We'll query the browser and ask it send capabilities.
var response = await context.BrowserRefreshServer.ReceiveAsync(buffer, cancellationToken);
if (!response.HasValue || !response.Value.EndOfMessage || response.Value.MessageType != WebSocketMessageType.Text)
{
return _baselineCapabilities;
throw new ApplicationException("Unable to connect to the browser refresh server.");
}

var values = Encoding.UTF8.GetString(buffer.AsSpan(0, response.Value.Count));
var capabilities = Encoding.UTF8.GetString(buffer.AsSpan(0, response.Value.Count));

// Capabilitiies are expressed a space-separated string.
// Capabilities are expressed a space-separated string.
// e.g. https://github.com/dotnet/runtime/blob/14343bdc281102bf6fffa1ecdd920221d46761bc/src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs#L87
var result = values.Split(' ').ToImmutableArray();
return result;
}
catch (TimeoutException)
{
return capabilities.Split(' ').ToImmutableArray();
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
}

return _baselineCapabilities;
}
}

public async ValueTask<bool> Apply(DotNetWatchContext context, ImmutableArray<WatchHotReloadService.Update> solutionUpdate, CancellationToken cancellationToken)
{
if (context.BrowserRefreshServer is null)
{
_reporter.Verbose("Unable to send deltas because the refresh server is unavailable.");
_reporter.Verbose("Unable to send deltas because the browser refresh server is unavailable.");
return false;
}

Expand All @@ -104,7 +92,7 @@ public async ValueTask<bool> Apply(DotNetWatchContext context, ImmutableArray<Wa
});

await context.BrowserRefreshServer.SendJsonWithSecret(sharedSecret => new UpdatePayload { SharedSecret = sharedSecret, Deltas = deltas }, cancellationToken);
return await VerifyDeltaApplied(context, cancellationToken).WaitAsync(VerifyDeltaTimeout, cancellationToken);
return await VerifyDeltaApplied(context, cancellationToken);
}

public async ValueTask ReportDiagnosticsAsync(DotNetWatchContext context, IEnumerable<string> diagnostics, CancellationToken cancellationToken)
Expand All @@ -123,33 +111,18 @@ public async ValueTask ReportDiagnosticsAsync(DotNetWatchContext context, IEnume
private async Task<bool> VerifyDeltaApplied(DotNetWatchContext context, CancellationToken cancellationToken)
{
var _receiveBuffer = new byte[1];
try
var result = await context.BrowserRefreshServer!.ReceiveAsync(_receiveBuffer, cancellationToken);
if (result is null)
{
// We want to give the client some time to ACK the deltas being applied. VerifyDeltaApplied is limited by a
// 5 second wait timeout enforced using a WaitAsync. However, WaitAsync only works reliably if the calling
// function is async. If BrowserRefreshServer.ReceiveAsync finishes synchronously, the WaitAsync would
// never have an opportunity to execute. Consequently, we'll give it some reasonable number of opportunities
// to loop before we decide that applying deltas failed.
for (var i = 0; i < 100; i++)
{
var result = await context.BrowserRefreshServer!.ReceiveAsync(_receiveBuffer, cancellationToken);
if (result is null)
{
// A null result indicates no clients are connected. No deltas could have been applied in this state.
_reporter.Verbose("No client is connected to ack deltas");
return false;
}

if (IsDeltaReceivedMessage(result.Value))
{
// 1 indicates success.
return _receiveBuffer[0] == 1;
}
}
// A null result indicates no clients are connected. No deltas could have been applied in this state.
_reporter.Verbose("No client is connected to ack deltas");
return false;
}
catch (TaskCanceledException)

if (IsDeltaReceivedMessage(result.Value))
{
_reporter.Verbose("Timed out while waiting to verify delta was applied.");
// 1 indicates success.
return _receiveBuffer[0] == 1;
}

return false;
Expand Down
36 changes: 15 additions & 21 deletions src/BuiltInTools/dotnet-watch/HotReload/CompilationWorkspaceProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,27 @@ static async void CreateProject(
}
else
{
taskCompletionSource.TrySetException(new InvalidOperationException($"Failed to create MSBuildWorkspace: {diag.Diagnostic}"));
taskCompletionSource.TrySetException(new ApplicationException($"Failed to create MSBuildWorkspace: {diag.Diagnostic}"));
}
};

await workspace.OpenProjectAsync(projectPath, cancellationToken: cancellationToken);
var currentSolution = workspace.CurrentSolution;

var hotReloadCapabilities = await GetHotReloadCapabilitiesAsync(hotReloadCapabilitiesTask, reporter);
var hotReloadService = new WatchHotReloadService(workspace.Services, await hotReloadCapabilitiesTask);
ImmutableArray<string> hotReloadCapabilities;
try
{
hotReloadCapabilities = await hotReloadCapabilitiesTask;
}
catch (Exception ex)
{
taskCompletionSource.TrySetException(new ApplicationException("Failed to read Hot Reload capabilities: " + ex.Message, ex));
return;
}

reporter.Verbose($"Hot reload capabilities: {string.Join(" ", hotReloadCapabilities)}.", emoji: "🔥");

var hotReloadService = new WatchHotReloadService(workspace.Services, hotReloadCapabilities);

await hotReloadService.StartSessionAsync(currentSolution, cancellationToken);

Expand All @@ -76,23 +88,5 @@ await Task.WhenAll(
taskCompletionSource.TrySetException(ex);
}
}

private static async Task<ImmutableArray<string>> GetHotReloadCapabilitiesAsync(Task<ImmutableArray<string>> hotReloadCapabilitiesTask, IReporter reporter)
{
try
{
var capabilities = await hotReloadCapabilitiesTask;
reporter.Verbose($"Hot reload capabilities: {string.Join(" ", capabilities)}.", emoji: "🔥");

return capabilities;
}
catch (Exception ex)
{
reporter.Verbose("Reading hot reload capabilities failed. Using default capabilities.");
reporter.Verbose(ex.ToString());

return ImmutableArray.Create("Baseline", "AddDefinitionToExistingType", "NewTypeDefinition");
}
}
}
}
43 changes: 13 additions & 30 deletions src/BuiltInTools/dotnet-watch/HotReload/DefaultDeltaApplier.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ internal class DefaultDeltaApplier : IDeltaApplier
{
private static readonly string _namedPipeName = Guid.NewGuid().ToString();
private readonly IReporter _reporter;
private Task? _connectionTask;
private Task<ImmutableArray<string>>? _capabilities;
private Task<ImmutableArray<string>>? _capabilitiesTask;
private NamedPipeServerStream? _pipe;

public DefaultDeltaApplier(IReporter reporter)
Expand All @@ -38,24 +37,16 @@ public ValueTask InitializeAsync(DotNetWatchContext context, CancellationToken c
if (!SuppressNamedPipeForTests)
{
_pipe = new NamedPipeServerStream(_namedPipeName, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous | PipeOptions.CurrentUserOnly);
_connectionTask = _pipe.WaitForConnectionAsync(cancellationToken);

_capabilities = Task.Run(async () =>
_capabilitiesTask = Task.Run(async () =>
{
try
{
await _connectionTask;
// When the client connects, the first payload it sends is the initialization payload which includes the apply capabilities.
var capabiltiies = ClientInitializationPayload.Read(_pipe).Capabilities;
_reporter.Verbose($"Application supports the following capabilities {capabiltiies}.");
return capabiltiies.Split(' ').ToImmutableArray();
}
catch
{
// Do nothing. This is awaited by Apply which will surface the error.
}
return ImmutableArray<string>.Empty;
_reporter.Verbose($"Connecting to the application.");
await _pipe.WaitForConnectionAsync(cancellationToken);
// When the client connects, the first payload it sends is the initialization payload which includes the apply capabilities.
var capabilities = ClientInitializationPayload.Read(_pipe).Capabilities;
return capabilities.Split(' ').ToImmutableArray();
});
}

Expand All @@ -72,11 +63,11 @@ public ValueTask InitializeAsync(DotNetWatchContext context, CancellationToken c
}

public Task<ImmutableArray<string>> GetApplyUpdateCapabilitiesAsync(DotNetWatchContext context, CancellationToken cancellationToken)
=> _capabilities ?? Task.FromResult(ImmutableArray<string>.Empty);
=> _capabilitiesTask ?? Task.FromResult(ImmutableArray<string>.Empty);

public async ValueTask<bool> Apply(DotNetWatchContext context, ImmutableArray<WatchHotReloadService.Update> solutionUpdate, CancellationToken cancellationToken)
{
if (_connectionTask is null || !_connectionTask.IsCompletedSuccessfully || _pipe is null || !_pipe.IsConnected)
if (_capabilitiesTask is null || !_capabilitiesTask.IsCompletedSuccessfully || _pipe is null || !_pipe.IsConnected)
{
// The client isn't listening
_reporter.Verbose("No client connected to receive delta updates.");
Expand All @@ -101,15 +92,7 @@ public async ValueTask<bool> Apply(DotNetWatchContext context, ImmutableArray<Wa
var bytes = ArrayPool<byte>.Shared.Rent(1);
try
{
var timeout =
#if DEBUG
Timeout.InfiniteTimeSpan;
#else
TimeSpan.FromSeconds(5);
#endif

using var cancellationTokenSource = new CancellationTokenSource(timeout);
var numBytes = await _pipe.ReadAsync(bytes, cancellationTokenSource.Token);
var numBytes = await _pipe.ReadAsync(bytes, cancellationToken);

if (numBytes == 1)
{
Expand Down

0 comments on commit ae921da

Please sign in to comment.