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

Remove various timeouts #27699

Merged
merged 4 commits into from
Sep 14, 2022
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
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
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);
tmat marked this conversation as resolved.
Show resolved Hide resolved
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");
tmat marked this conversation as resolved.
Show resolved Hide resolved
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
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
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