Skip to content

Commit ecdfff3

Browse files
stephentoubCopilot
andauthored
Add graceful runtime shutdown to SDK clients (#1539)
* Add graceful runtime shutdown to SDK clients Call runtime.shutdown during normal client stop for SDK-owned CLI processes across all language SDKs. Add bounded cleanup, timing diagnostics, and lifecycle coverage while preserving force-stop and external-runtime behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address graceful shutdown review feedback Narrow .NET shutdown exception handling, avoid blocking Python's event loop while waiting for CLI process exit, clear external force-stop process references, and apply Java formatting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3eaccba commit ecdfff3

16 files changed

Lines changed: 978 additions & 226 deletions

File tree

dotnet/src/Client.cs

Lines changed: 80 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public sealed partial class CopilotClient : IDisposable, IAsyncDisposable
5858
/// </summary>
5959
private const int MinProtocolVersion = 3;
6060
private static readonly TimeSpan s_stderrPumpShutdownTimeout = TimeSpan.FromSeconds(5);
61+
private static readonly TimeSpan s_runtimeShutdownTimeout = TimeSpan.FromSeconds(10);
6162

6263
/// <summary>
6364
/// Provides a thread-safe collection of active Copilot sessions, indexed by session identifier.
@@ -291,7 +292,7 @@ async Task<Connection> StartCoreAsync(CancellationToken ct)
291292

292293
if (connection is not null)
293294
{
294-
await CleanupConnectionAsync(connection, errors: null);
295+
await CleanupConnectionAsync(connection, errors: null, gracefulRuntimeShutdown: false);
295296
}
296297
else if (cliProcess is not null)
297298
{
@@ -312,6 +313,7 @@ async Task<Connection> StartCoreAsync(CancellationToken ct)
312313
/// This method performs graceful cleanup:
313314
/// <list type="number">
314315
/// <item>Closes all active sessions (releases in-memory resources)</item>
316+
/// <item>Requests runtime shutdown for SDK-owned CLI processes</item>
315317
/// <item>Closes the JSON-RPC connection</item>
316318
/// <item>Terminates the CLI server process (if spawned by this client)</item>
317319
/// </list>
@@ -346,7 +348,7 @@ public async Task StopAsync()
346348

347349
_sessions.Clear();
348350

349-
await CleanupConnectionAsync(errors);
351+
await CleanupConnectionAsync(errors, gracefulRuntimeShutdown: true);
350352

351353
ThrowErrors(errors);
352354
}
@@ -378,7 +380,7 @@ public async Task ForceStopAsync()
378380
_sessions.Clear();
379381

380382
var errors = new List<Exception>();
381-
await CleanupConnectionAsync(errors);
383+
await CleanupConnectionAsync(errors, gracefulRuntimeShutdown: false);
382384
ThrowErrors(errors);
383385
}
384386

@@ -398,7 +400,7 @@ private static void ThrowErrors(List<Exception>? errors)
398400
}
399401
}
400402

401-
private async Task CleanupConnectionAsync(List<Exception>? errors)
403+
private async Task CleanupConnectionAsync(List<Exception>? errors, bool gracefulRuntimeShutdown)
402404
{
403405
var connectionTask = _connectionTask;
404406
if (connectionTask is null)
@@ -419,11 +421,36 @@ private async Task CleanupConnectionAsync(List<Exception>? errors)
419421
return;
420422
}
421423

422-
await CleanupConnectionAsync(ctx, errors);
424+
await CleanupConnectionAsync(ctx, errors, gracefulRuntimeShutdown);
423425
}
424426

425-
private async Task CleanupConnectionAsync(Connection ctx, List<Exception>? errors)
427+
private async Task CleanupConnectionAsync(Connection ctx, List<Exception>? errors, bool gracefulRuntimeShutdown)
426428
{
429+
var runtimeShutdownCompleted = false;
430+
if (gracefulRuntimeShutdown && ctx.CliProcess is not null)
431+
{
432+
var runtimeShutdownTimestamp = Stopwatch.GetTimestamp();
433+
try
434+
{
435+
using var cancellation = new CancellationTokenSource(s_runtimeShutdownTimeout);
436+
await ctx.Server.Runtime.ShutdownAsync(cancellation.Token);
437+
runtimeShutdownCompleted = true;
438+
LoggingHelpers.LogTiming(_logger, LogLevel.Debug, null,
439+
"CopilotClient.StopAsync runtime shutdown complete. Elapsed={Elapsed}",
440+
runtimeShutdownTimestamp);
441+
}
442+
catch (Exception ex) when (ex is OperationCanceledException
443+
or InvalidOperationException
444+
or ObjectDisposedException
445+
or IOException
446+
or SocketException)
447+
{
448+
LoggingHelpers.LogTiming(_logger, LogLevel.Debug, ex,
449+
"CopilotClient.StopAsync runtime shutdown failed. Elapsed={Elapsed}",
450+
runtimeShutdownTimestamp);
451+
}
452+
}
453+
427454
try { ctx.Rpc.Dispose(); }
428455
catch (Exception ex) { AddCleanupError(errors, ex, _logger); }
429456

@@ -439,22 +466,62 @@ private async Task CleanupConnectionAsync(Connection ctx, List<Exception>? error
439466

440467
if (ctx.CliProcess is { } childProcess)
441468
{
442-
await CleanupCliProcessAsync(childProcess, ctx.StderrPump, errors, _logger);
469+
await CleanupCliProcessAsync(childProcess, ctx.StderrPump, errors, _logger, runtimeShutdownCompleted);
443470
}
444471
}
445472

446-
private static async Task CleanupCliProcessAsync(Process childProcess, ProcessStderrPump? stderrPump, List<Exception>? errors, ILogger? logger)
473+
private static async Task CleanupCliProcessAsync(Process childProcess, ProcessStderrPump? stderrPump, List<Exception>? errors, ILogger? logger, bool waitForGracefulExit = false)
447474
{
448475
stderrPump?.Cancel();
449476

450477
try
451478
{
452479
if (!childProcess.HasExited)
453480
{
481+
if (waitForGracefulExit)
482+
{
483+
var shutdownWaitTimestamp = Stopwatch.GetTimestamp();
484+
try
485+
{
486+
await childProcess.WaitForExitAsync().WaitAsync(s_runtimeShutdownTimeout);
487+
}
488+
catch (TimeoutException ex)
489+
{
490+
if (logger is not null)
491+
{
492+
LoggingHelpers.LogTiming(logger, LogLevel.Debug, ex,
493+
"Timed out waiting for runtime process to exit after graceful shutdown. Elapsed={Elapsed}, Timeout={Timeout}",
494+
shutdownWaitTimestamp,
495+
s_runtimeShutdownTimeout);
496+
}
497+
}
498+
}
499+
500+
if (childProcess.HasExited)
501+
{
502+
return;
503+
}
504+
454505
childProcess.Kill(entireProcessTree: true);
455506
// Kill is asynchronous; wait for the root CLI process to exit so cleanup callers
456507
// do not observe StopAsync/DisposeAsync completion while it is still tearing down.
457-
await childProcess.WaitForExitAsync();
508+
var killWaitTimestamp = Stopwatch.GetTimestamp();
509+
try
510+
{
511+
await childProcess.WaitForExitAsync().WaitAsync(s_runtimeShutdownTimeout);
512+
}
513+
catch (TimeoutException ex)
514+
{
515+
if (logger is not null)
516+
{
517+
LoggingHelpers.LogTiming(logger, LogLevel.Debug, ex,
518+
"Timed out waiting for runtime process to exit after kill. Elapsed={Elapsed}, Timeout={Timeout}",
519+
killWaitTimestamp,
520+
s_runtimeShutdownTimeout);
521+
}
522+
523+
AddCleanupError(errors, ex, logger);
524+
}
458525
}
459526
}
460527
catch (Exception ex)
@@ -2002,9 +2069,10 @@ private async Task<Connection> ConnectToServerAsync(Process? cliProcess, string?
20022069
"CopilotClient.ConnectToServerAsync transport setup complete. Elapsed={Elapsed}",
20032070
setupTimestamp);
20042071

2005-
_serverRpc = new ServerRpc(rpc);
2072+
var connection = new Connection(rpc, cliProcess, networkStream, stderrPump);
2073+
_serverRpc = connection.Server;
20062074

2007-
return new Connection(rpc, cliProcess, networkStream, stderrPump);
2075+
return connection;
20082076
}
20092077
catch
20102078
{
@@ -2208,6 +2276,7 @@ private class Connection(
22082276
{
22092277
public Process? CliProcess => cliProcess;
22102278
public JsonRpc Rpc => rpc;
2279+
public ServerRpc Server => field ?? Interlocked.CompareExchange(ref field, new(rpc), null) ?? field;
22112280
public NetworkStream? NetworkStream => networkStream;
22122281
public ProcessStderrPump? StderrPump => stderrPump;
22132282
public StringBuilder? StderrBuffer => stderrPump?.Buffer;

dotnet/test/E2E/TelemetryExportE2ETests.cs

Lines changed: 12 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,7 @@ public async Task Should_Export_File_Telemetry_For_Sdk_Interactions()
4747
await session.DisposeAsync();
4848
await client.StopAsync();
4949

50-
var entries = await ReadTelemetryEntriesAsync(
51-
telemetryPath,
52-
entries => entries.Any(entry => GetTypeName(entry) == "span" &&
53-
GetStringAttribute(entry, "gen_ai.operation.name") == "invoke_agent"));
50+
var entries = await ReadTelemetryEntriesAsync(telemetryPath);
5451
var spans = entries.Where(entry => GetTypeName(entry) == "span").ToList();
5552

5653
Assert.NotEmpty(spans);
@@ -89,46 +86,23 @@ public async Task Should_Export_File_Telemetry_For_Sdk_Interactions()
8986
static string EchoTelemetryMarker(string value) => value;
9087
}
9188

92-
private static async Task<IReadOnlyList<JsonElement>> ReadTelemetryEntriesAsync(
93-
string path,
94-
Func<IReadOnlyList<JsonElement>, bool> isComplete)
89+
private static async Task<IReadOnlyList<JsonElement>> ReadTelemetryEntriesAsync(string path)
9590
{
96-
IReadOnlyList<JsonElement> entries = [];
97-
await TestHelper.WaitForConditionAsync(
98-
async () =>
99-
{
100-
entries = await ReadTelemetryEntriesOnceAsync(path);
101-
return entries.Count > 0 && isComplete(entries);
102-
},
103-
timeout: TimeSpan.FromSeconds(30),
104-
timeoutMessage: $"Timed out waiting for telemetry records in '{path}'.",
105-
transientExceptionFilter: exception => TestHelper.IsTransientFileSystemException(exception) || exception is JsonException);
106-
107-
return entries;
108-
109-
static async Task<IReadOnlyList<JsonElement>> ReadTelemetryEntriesOnceAsync(string path)
91+
var entries = new List<JsonElement>();
92+
using var stream = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read);
93+
using var reader = new StreamReader(stream);
94+
while (await reader.ReadLineAsync() is { } line)
11095
{
111-
if (!File.Exists(path) || new FileInfo(path).Length == 0)
96+
if (string.IsNullOrWhiteSpace(line))
11297
{
113-
return [];
98+
continue;
11499
}
115100

116-
var entries = new List<JsonElement>();
117-
using var stream = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete);
118-
using var reader = new StreamReader(stream);
119-
while (await reader.ReadLineAsync() is { } line)
120-
{
121-
if (string.IsNullOrWhiteSpace(line))
122-
{
123-
continue;
124-
}
125-
126-
using var document = JsonDocument.Parse(line);
127-
entries.Add(document.RootElement.Clone());
128-
}
129-
130-
return entries;
101+
using var document = JsonDocument.Parse(line);
102+
entries.Add(document.RootElement.Clone());
131103
}
104+
105+
return entries;
132106
}
133107

134108
private static string? GetTraceId(JsonElement entry) => GetStringProperty(entry, "traceId");

0 commit comments

Comments
 (0)