Skip to content

Commit 30b1fb4

Browse files
tmattmat
committed
Notify DCP of terminated session when process exits on its own
1 parent 2ab81d4 commit 30b1fb4

File tree

27 files changed

+433
-231
lines changed

27 files changed

+433
-231
lines changed

‎src/BuiltInTools/AspireService/AspireServerService.cs‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ public List<KeyValuePair<string, string>> GetServerConnectionEnvironment()
123123
new(DebugSessionServerCertEnvVar, _certificateEncodedBytes),
124124
];
125125

126+
/// <exception cref="OperationCanceledException"/>
126127
public ValueTask NotifySessionEndedAsync(string dcpId, string sessionId, int processId, int? exitCode, CancellationToken cancelationToken)
127128
=> SendNotificationAsync(
128129
new SessionTerminatedNotification()
@@ -136,6 +137,7 @@ public ValueTask NotifySessionEndedAsync(string dcpId, string sessionId, int pro
136137
sessionId,
137138
cancelationToken);
138139

140+
/// <exception cref="OperationCanceledException"/>
139141
public ValueTask NotifySessionStartedAsync(string dcpId, string sessionId, int processId, CancellationToken cancelationToken)
140142
=> SendNotificationAsync(
141143
new ProcessRestartedNotification()
@@ -148,6 +150,7 @@ public ValueTask NotifySessionStartedAsync(string dcpId, string sessionId, int p
148150
sessionId,
149151
cancelationToken);
150152

153+
/// <exception cref="OperationCanceledException"/>
151154
public ValueTask NotifyLogMessageAsync(string dcpId, string sessionId, bool isStdErr, string data, CancellationToken cancelationToken)
152155
=> SendNotificationAsync(
153156
new ServiceLogsNotification()
@@ -161,23 +164,28 @@ public ValueTask NotifyLogMessageAsync(string dcpId, string sessionId, bool isSt
161164
sessionId,
162165
cancelationToken);
163166

164-
private async ValueTask SendNotificationAsync<TNotification>(TNotification notification, string dcpId, string sessionId, CancellationToken cancelationToken)
167+
/// <exception cref="OperationCanceledException"/>
168+
private async ValueTask SendNotificationAsync<TNotification>(TNotification notification, string dcpId, string sessionId, CancellationToken cancellationToken)
165169
where TNotification : SessionNotification
166170
{
167171
try
168172
{
169-
Log($"[#{sessionId}] Sending '{notification.NotificationType}'");
173+
Log($"[#{sessionId}] Sending '{notification.NotificationType}': {notification}");
170174
var jsonSerialized = JsonSerializer.SerializeToUtf8Bytes(notification, JsonSerializerOptions);
171-
await SendMessageAsync(dcpId, jsonSerialized, cancelationToken);
172-
}
173-
catch (Exception e) when (e is not OperationCanceledException && LogAndPropagate(e))
174-
{
175-
}
175+
var success = await SendMessageAsync(dcpId, jsonSerialized, cancellationToken);
176176

177-
bool LogAndPropagate(Exception e)
177+
if (!success)
178+
{
179+
cancellationToken.ThrowIfCancellationRequested();
180+
Log($"[#{sessionId}] Failed to send message: Connection not found (dcpId='{dcpId}').");
181+
}
182+
}
183+
catch (Exception e) when (e is not OperationCanceledException)
178184
{
179-
Log($"[#{sessionId}] Sending '{notification.NotificationType}' failed: {e.Message}");
180-
return false;
185+
if (!cancellationToken.IsCancellationRequested)
186+
{
187+
Log($"[#{sessionId}] Failed to send message: {e.Message}");
188+
}
181189
}
182190
}
183191

@@ -373,15 +381,13 @@ private async Task WriteResponseTextAsync(HttpResponse response, Exception ex, b
373381
}
374382
}
375383

376-
private async Task SendMessageAsync(string dcpId, byte[] messageBytes, CancellationToken cancellationToken)
384+
private async ValueTask<bool> SendMessageAsync(string dcpId, byte[] messageBytes, CancellationToken cancellationToken)
377385
{
378386
// Find the connection for the passed in dcpId
379387
WebSocketConnection? connection = _socketConnectionManager.GetSocketConnection(dcpId);
380388
if (connection is null)
381389
{
382-
// Most likely the connection has already gone away
383-
Log($"Send message failure: Connection with the following dcpId was not found {dcpId}");
384-
return;
390+
return false;
385391
}
386392

387393
var success = false;
@@ -405,6 +411,8 @@ private async Task SendMessageAsync(string dcpId, byte[] messageBytes, Cancellat
405411

406412
_webSocketAccess.Release();
407413
}
414+
415+
return success;
408416
}
409417

410418
private async ValueTask HandleStopSessionRequestAsync(HttpContext context, string sessionId)

‎src/BuiltInTools/AspireService/Models/SessionChangeNotification.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ internal sealed class SessionTerminatedNotification : SessionNotification
5656
[Required]
5757
[JsonPropertyName("exit_code")]
5858
public required int? ExitCode { get; init; }
59+
60+
public override string ToString()
61+
=> $"pid={Pid}, exit_code={ExitCode}";
5962
}
6063

6164
/// <summary>
@@ -70,6 +73,9 @@ internal sealed class ProcessRestartedNotification : SessionNotification
7073
[Required]
7174
[JsonPropertyName("pid")]
7275
public required int PID { get; init; }
76+
77+
public override string ToString()
78+
=> $"pid={PID}";
7379
}
7480

7581
/// <summary>
@@ -91,4 +97,7 @@ internal sealed class ServiceLogsNotification : SessionNotification
9197
[Required]
9298
[JsonPropertyName("log_message")]
9399
public required string LogMessage { get; init; }
100+
101+
public override string ToString()
102+
=> $"log_message='{LogMessage}', is_std_err={IsStdErr}";
94103
}

‎src/BuiltInTools/HotReloadClient/DefaultHotReloadClient.cs‎

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

3636
private void DisposePipe()
3737
{
38-
Logger.LogDebug("Disposing agent communication pipe");
39-
_pipe?.Dispose();
40-
_pipe = null;
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+
}
4146
}
4247

4348
// for testing
@@ -101,8 +106,7 @@ private void RequireReadyForUpdates()
101106
// should only be called after connection has been created:
102107
_ = GetCapabilitiesTask();
103108

104-
if (_pipe == null)
105-
throw new InvalidOperationException("Pipe has been disposed.");
109+
Debug.Assert(_pipe != null);
106110
}
107111

108112
public override void ConfigureLaunchEnvironment(IDictionary<string, string> environmentBuilder)
@@ -152,7 +156,13 @@ public override async Task<ApplyStatus> ApplyManagedCodeUpdatesAsync(ImmutableAr
152156
{
153157
if (!success)
154158
{
155-
Logger.LogWarning("Further changes won't be applied to this process.");
159+
// 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.
161+
if (!cancellationToken.IsCancellationRequested)
162+
{
163+
Logger.LogWarning("Further changes won't be applied to this process.");
164+
}
165+
156166
_managedCodeUpdateFailedOrCancelled = true;
157167
DisposePipe();
158168
}
@@ -216,7 +226,7 @@ public async override Task<ApplyStatus> ApplyStaticAssetUpdatesAsync(ImmutableAr
216226
private ValueTask<bool> SendAndReceiveUpdateAsync<TRequest>(TRequest request, bool isProcessSuspended, CancellationToken cancellationToken)
217227
where TRequest : IUpdateRequest
218228
{
219-
// Should not be disposed:
229+
// Should not initialized:
220230
Debug.Assert(_pipe != null);
221231

222232
return SendAndReceiveUpdateAsync(
@@ -241,8 +251,10 @@ async ValueTask<bool> SendAndReceiveAsync(int batchId, CancellationToken cancell
241251

242252
Logger.LogDebug("Update batch #{UpdateId} failed.", batchId);
243253
}
244-
catch (Exception e) when (e is not OperationCanceledException || isProcessSuspended)
254+
catch (Exception e)
245255
{
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.
246258
if (cancellationToken.IsCancellationRequested)
247259
{
248260
Logger.LogDebug("Update batch #{UpdateId} canceled.", batchId);
@@ -267,7 +279,7 @@ async ValueTask WriteRequestAsync(CancellationToken cancellationToken)
267279

268280
private async ValueTask<bool> ReceiveUpdateResponseAsync(CancellationToken cancellationToken)
269281
{
270-
// Should not be disposed:
282+
// Should be initialized:
271283
Debug.Assert(_pipe != null);
272284

273285
var (success, log) = await UpdateResponse.ReadAsync(_pipe, cancellationToken);
@@ -296,10 +308,12 @@ public override async Task InitialUpdatesAppliedAsync(CancellationToken cancella
296308
}
297309
catch (Exception e) when (e is not OperationCanceledException)
298310
{
299-
// 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.
300314
if (!cancellationToken.IsCancellationRequested)
301315
{
302-
Logger.LogError("Failed to send InitialUpdatesCompleted: {Message}", e.Message);
316+
Logger.LogError("Failed to send {RequestType}: {Message}", nameof(RequestType.InitialUpdatesCompleted), e.Message);
303317
}
304318
}
305319
}

‎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
}

0 commit comments

Comments
 (0)