Skip to content

Commit 5dc2950

Browse files
tmattmat
committed
Fixes
1 parent ccecbde commit 5dc2950

File tree

6 files changed

+67
-26
lines changed

6 files changed

+67
-26
lines changed

‎src/BuiltInTools/HotReloadClient/DefaultHotReloadClient.cs‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,14 @@ public override void Dispose()
3535

3636
private void DisposePipe()
3737
{
38-
Logger.LogDebug("Disposing agent communication pipe");
39-
_pipe?.Dispose();
38+
if (_pipe != null)
39+
{
40+
Logger.LogDebug("Disposing agent communication pipe");
41+
42+
// Dispose the pipe but do not set it to null, so that any in-progress
43+
// operations throw the appropriate exception type.
44+
_pipe.Dispose();
45+
}
4046
}
4147

4248
// for testing
@@ -151,6 +157,7 @@ public override async Task<ApplyStatus> ApplyManagedCodeUpdatesAsync(ImmutableAr
151157
if (!success)
152158
{
153159
// Don't report a warning when cancelled. The process has terminated or the host is shutting down in that case.
160+
// Best effort: There is an inherent race condition due to time between the process exiting and the cancellation token triggering.
154161
if (!cancellationToken.IsCancellationRequested)
155162
{
156163
Logger.LogWarning("Further changes won't be applied to this process.");
@@ -246,6 +253,8 @@ async ValueTask<bool> SendAndReceiveAsync(int batchId, CancellationToken cancell
246253
}
247254
catch (Exception e)
248255
{
256+
// Don't report an error when cancelled. The process has terminated or the host is shutting down in that case.
257+
// Best effort: There is an inherent race condition due to time between the process exiting and the cancellation token triggering.
249258
if (cancellationToken.IsCancellationRequested)
250259
{
251260
Logger.LogDebug("Update batch #{UpdateId} canceled.", batchId);
@@ -299,7 +308,9 @@ public override async Task InitialUpdatesAppliedAsync(CancellationToken cancella
299308
}
300309
catch (Exception e) when (e is not OperationCanceledException)
301310
{
302-
// pipe might throw another exception when forcibly closed on process termination:
311+
// Pipe might throw another exception when forcibly closed on process termination.
312+
// Don't report an error when cancelled. The process has terminated or the host is shutting down in that case.
313+
// Best effort: There is an inherent race condition due to time between the process exiting and the cancellation token triggering.
303314
if (!cancellationToken.IsCancellationRequested)
304315
{
305316
Logger.LogError("Failed to send {RequestType}: {Message}", nameof(RequestType.InitialUpdatesCompleted), e.Message);

‎src/BuiltInTools/HotReloadClient/HotReloadClient.cs‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,23 +41,42 @@ internal Task PendingUpdates
4141
/// <summary>
4242
/// Initiates connection with the agent in the target process.
4343
/// </summary>
44+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
4445
public abstract void InitiateConnection(CancellationToken cancellationToken);
4546

4647
/// <summary>
4748
/// Waits until the connection with the agent is established.
4849
/// </summary>
50+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
4951
public abstract Task WaitForConnectionEstablishedAsync(CancellationToken cancellationToken);
5052

53+
/// <summary>
54+
/// Returns update capabilities of the target process.
55+
/// </summary>
56+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
5157
public abstract Task<ImmutableArray<string>> GetUpdateCapabilitiesAsync(CancellationToken cancellationToken);
5258

59+
/// <summary>
60+
/// Applies managed code updates to the target process.
61+
/// </summary>
62+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
5363
public abstract Task<ApplyStatus> ApplyManagedCodeUpdatesAsync(ImmutableArray<HotReloadManagedCodeUpdate> updates, bool isProcessSuspended, CancellationToken cancellationToken);
64+
65+
/// <summary>
66+
/// Applies static asset updates to the target process.
67+
/// </summary>
68+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
5469
public abstract Task<ApplyStatus> ApplyStaticAssetUpdatesAsync(ImmutableArray<HotReloadStaticAssetUpdate> updates, bool isProcessSuspended, CancellationToken cancellationToken);
5570

5671
/// <summary>
5772
/// Notifies the agent that the initial set of updates has been applied and the user code in the process can start executing.
5873
/// </summary>
74+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
5975
public abstract Task InitialUpdatesAppliedAsync(CancellationToken cancellationToken);
6076

77+
/// <summary>
78+
/// Disposes the client. Can occur unexpectedly whenever the process exits.
79+
/// </summary>
6180
public abstract void Dispose();
6281

6382
public static void ReportLogEntry(ILogger logger, string message, AgentMessageSeverity severity)
@@ -72,7 +91,7 @@ public static void ReportLogEntry(ILogger logger, string message, AgentMessageSe
7291
logger.Log(level, message);
7392
}
7493

75-
public async Task<IReadOnlyList<HotReloadManagedCodeUpdate>> FilterApplicableUpdatesAsync(ImmutableArray<HotReloadManagedCodeUpdate> updates, CancellationToken cancellationToken)
94+
protected async Task<IReadOnlyList<HotReloadManagedCodeUpdate>> FilterApplicableUpdatesAsync(ImmutableArray<HotReloadManagedCodeUpdate> updates, CancellationToken cancellationToken)
7695
{
7796
var availableCapabilities = await GetUpdateCapabilitiesAsync(cancellationToken);
7897
var applicableUpdates = new List<HotReloadManagedCodeUpdate>();

‎src/BuiltInTools/HotReloadClient/HotReloadClients.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ public HotReloadClients(HotReloadClient client, AbstractBrowserRefreshServer? br
2323
{
2424
}
2525

26+
/// <summary>
27+
/// Disposes all clients. Can occur unexpectedly whenever the process exits.
28+
/// </summary>
2629
public void Dispose()
2730
{
2831
foreach (var (client, _) in clients)
@@ -56,6 +59,7 @@ internal void ConfigureLaunchEnvironment(IDictionary<string, string> environment
5659
browserRefreshServer?.ConfigureLaunchEnvironment(environmentBuilder, enableHotReload: true);
5760
}
5861

62+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
5963
internal void InitiateConnection(CancellationToken cancellationToken)
6064
{
6165
foreach (var (client, _) in clients)
@@ -64,11 +68,13 @@ internal void InitiateConnection(CancellationToken cancellationToken)
6468
}
6569
}
6670

71+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
6772
internal async ValueTask WaitForConnectionEstablishedAsync(CancellationToken cancellationToken)
6873
{
6974
await Task.WhenAll(clients.Select(c => c.client.WaitForConnectionEstablishedAsync(cancellationToken)));
7075
}
7176

77+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
7278
public async ValueTask<ImmutableArray<string>> GetUpdateCapabilitiesAsync(CancellationToken cancellationToken)
7379
{
7480
if (clients is [var (singleClient, _)])
@@ -83,6 +89,7 @@ public async ValueTask<ImmutableArray<string>> GetUpdateCapabilitiesAsync(Cancel
8389
return [.. results.SelectMany(r => r).Distinct(StringComparer.Ordinal).OrderBy(c => c)];
8490
}
8591

92+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
8693
public async ValueTask ApplyManagedCodeUpdatesAsync(ImmutableArray<HotReloadManagedCodeUpdate> updates, bool isProcessSuspended, CancellationToken cancellationToken)
8794
{
8895
var anyFailure = false;
@@ -139,6 +146,7 @@ public async ValueTask ApplyManagedCodeUpdatesAsync(ImmutableArray<HotReloadMana
139146
}
140147
}
141148

149+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
142150
public async ValueTask InitialUpdatesAppliedAsync(CancellationToken cancellationToken)
143151
{
144152
if (clients is [var (singleClient, _)])
@@ -151,6 +159,7 @@ public async ValueTask InitialUpdatesAppliedAsync(CancellationToken cancellation
151159
}
152160
}
153161

162+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
154163
public async Task ApplyStaticAssetUpdatesAsync(IEnumerable<(string filePath, string relativeUrl, string assemblyName, bool isApplicationProject)> assets, CancellationToken cancellationToken)
155164
{
156165
if (browserRefreshServer != null)
@@ -190,6 +199,7 @@ public async Task ApplyStaticAssetUpdatesAsync(IEnumerable<(string filePath, str
190199
}
191200
}
192201

202+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
193203
public async ValueTask ApplyStaticAssetUpdatesAsync(ImmutableArray<HotReloadStaticAssetUpdate> updates, bool isProcessSuspended, CancellationToken cancellationToken)
194204
{
195205
if (clients is [var (singleClient, _)])
@@ -202,6 +212,7 @@ public async ValueTask ApplyStaticAssetUpdatesAsync(ImmutableArray<HotReloadStat
202212
}
203213
}
204214

215+
/// <param name="cancellationToken">Cancellation token. The cancellation should trigger on process terminatation.</param>
205216
public ValueTask ReportCompilationErrorsInApplicationAsync(ImmutableArray<string> compilationErrors, CancellationToken cancellationToken)
206217
=> browserRefreshServer?.ReportCompilationErrorsInBrowserAsync(compilationErrors, cancellationToken) ?? ValueTask.CompletedTask;
207218
}

‎src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,8 @@ public async ValueTask<bool> HandleStaticAssetChangesAsync(IReadOnlyList<Changed
548548
var tasks = updates.Select(async entry =>
549549
{
550550
var (runningProject, assets) = entry;
551-
await runningProject.Clients.ApplyStaticAssetUpdatesAsync(assets, cancellationToken);
551+
using var processCommunicationCancellationSource = CancellationTokenSource.CreateLinkedTokenSource(runningProject.ProcessExitedCancellationToken, cancellationToken);
552+
await runningProject.Clients.ApplyStaticAssetUpdatesAsync(assets, processCommunicationCancellationSource.Token);
552553
});
553554

554555
await Task.WhenAll(tasks).WaitAsync(cancellationToken);

‎src/BuiltInTools/dotnet-watch/HotReload/HotReloadDotNetWatcher.cs‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,7 @@ public async Task WatchAsync(CancellationToken shutdownCancellationToken)
147147
return;
148148
}
149149

150-
try
151-
{
152-
await rootRunningProject.WaitForProcessRunningAsync(iterationCancellationToken);
153-
}
154-
catch (OperationCanceledException) when (rootRunningProject.ProcessExitedCancellationToken.IsCancellationRequested)
150+
if (!await rootRunningProject.WaitForProcessRunningAsync(iterationCancellationToken))
155151
{
156152
// Process might have exited while we were trying to communicate with it.
157153
// Cancel the iteration, but wait for a file change before starting a new one.
@@ -385,19 +381,7 @@ await Task.WhenAll(
385381
projectsToRestart.Select(async runningProject =>
386382
{
387383
var newRunningProject = await runningProject.RestartOperation(shutdownCancellationToken);
388-
389-
try
390-
{
391-
await newRunningProject.WaitForProcessRunningAsync(shutdownCancellationToken);
392-
}
393-
catch (OperationCanceledException) when (!shutdownCancellationToken.IsCancellationRequested)
394-
{
395-
// Process might have exited while we were trying to communicate with it.
396-
}
397-
finally
398-
{
399-
runningProject.Dispose();
400-
}
384+
_ = await newRunningProject.WaitForProcessRunningAsync(shutdownCancellationToken);
401385
}))
402386
.WaitAsync(shutdownCancellationToken);
403387

‎src/BuiltInTools/dotnet-watch/Process/RunningProject.cs‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,11 @@ internal sealed class RunningProject(
4444
public bool IsRestarting { get; private set; }
4545

4646
private volatile bool _isDisposed;
47-
47+
48+
/// <summary>
49+
/// Disposes the project. Can occur unexpectedly whenever the process exits.
50+
/// Must only be called once per project.
51+
/// </summary>
4852
public void Dispose()
4953
{
5054
ObjectDisposedException.ThrowIf(_isDisposed, this);
@@ -60,10 +64,21 @@ public void Dispose()
6064
/// <summary>
6165
/// Waits for the application process to start.
6266
/// Ensures that the build has been complete and the build outputs are available.
67+
/// Returns false if the process has exited before the connection was established.
6368
/// </summary>
64-
public async ValueTask WaitForProcessRunningAsync(CancellationToken cancellationToken)
69+
public async ValueTask<bool> WaitForProcessRunningAsync(CancellationToken cancellationToken)
6570
{
66-
await Clients.WaitForConnectionEstablishedAsync(cancellationToken);
71+
using var processCommunicationCancellationSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, ProcessExitedCancellationToken);
72+
73+
try
74+
{
75+
await Clients.WaitForConnectionEstablishedAsync(processCommunicationCancellationSource.Token);
76+
return true;
77+
}
78+
catch (OperationCanceledException) when (ProcessExitedCancellationToken.IsCancellationRequested)
79+
{
80+
return false;
81+
}
6782
}
6883

6984
public async Task<int> TerminateAsync(bool isRestarting)

0 commit comments

Comments
 (0)