[v2] Bound in-process FFI host_shutdown and fix .NET lifecycle race across all SDKs

.github/workflows/dotnet-sdk-tests.yml

@@ -56,20 +56,26 @@ jobs:
         transport: ["default", "inprocess"]
         backend: [capi]
         shard: [full]
-        # TODO: Re-enable after fixing in-process sqlite file locking on shutdown on Windows.
         exclude:
-          - os: windows-latest
-            transport: "inprocess"
           - os: windows-latest
             transport: default
             shard: full
           # TODO(cli-1.0.81-2): CLI 1.0.81-5 still stops completing in-process
           # CAPI model turns, causing repeated per-test timeouts until the
           # 30-minute job limit. Stdio CAPI and in-process BYOK remain enabled.
+          # This affects every OS equally (it is a CLI/CAPI regression, not a
+          # platform-specific one), so Linux/macOS are excluded from the `capi`
+          # in-process cell here; see the ubuntu-latest/inprocess include cells
+          # further down for their in-process coverage via the alternate
+          # backends. windows-latest/inprocess has no in-process coverage at
+          # all right now (capi or otherwise) -- see the comment further down
+          # by the removed windows-latest/inprocess include cells for why.
           - os: ubuntu-latest
             transport: inprocess
           - os: macos-latest
             transport: inprocess
+          - os: windows-latest
+            transport: inprocess
           # The macOS default/capi host runs the whole suite on the smallest
           # runner in the matrix (3 vCPU / 7 GB vs ubuntu's 4 / 16). Since the
           # 1.0.81-2 bump it stopped finishing: the job ran 50+ minutes until
@@ -182,11 +188,24 @@ jobs:
             backend: openai-completions
             shard: full
             test-filter: "FullyQualifiedName~GitHub.Copilot.Test.E2E&E2EBackend!=SelfConfiguredBackend&E2EBackend!=CapiOnly"
+          # Windows in-process coverage was attempted here (github/copilot-sdk#2525):
+          # FfiRuntimeHost.Dispose() now bounds its wait on native shutdown (see below),
+          # which should have made this safe to enable now that napi-oop is gone. But
+          # actually running it on real Windows CI (github/copilot-sdk#2531) reproduced
+          # native `System.AccessViolationException` crashes in ConnectionWrite during
+          # ordinary connection I/O -- unrelated to shutdown/disposal, and independently
+          # matched by a SIGSEGV in the Rust SDK's own Windows in-process CI in the same
+          # PR. That rules out an SDK-side binding bug; tracked upstream at
+          # github/copilot-agent-runtime#18990. Retrying after rebasing onto CLI 1.0.84-4
+          # reproduced the same blocker, so re-add windows-latest here once resolved.
     runs-on: ${{ matrix.os }}
     # A hung test used to run until the runner died (~50 min) and the dying
     # runner never uploaded its logs, so the failures were undiagnosable.
-    # Every healthy cell finishes well under 15 min.
-    timeout-minutes: 20
+    # Most healthy cells finish well under 15 min, but macOS shard 1 can run
+    # longer on the smallest runner after runtime/dependency updates. Keep the
+    # job bound high enough that slow-but-healthy shards aren't canceled without
+    # diagnostics.
+    timeout-minutes: 30
     defaults:
       run:
         shell: bash

.github/workflows/rust-sdk-tests.yml

@@ -20,7 +20,14 @@ jobs:
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
     runs-on: ${{ matrix.os }}
-    timeout-minutes: 20
+    # The "cargo test" step below allows up to 90 minutes on its own
+    # (timeout-minutes: 90), but a *job*-level timeout still cancels the whole
+    # job (including checkout/toolchain/cache steps) once it elapses,
+    # regardless of any step-level timeout. It must stay >= the step timeout
+    # plus setup overhead, or a slow-but-healthy run (e.g. a cold Windows
+    # dependency compile) is killed as "canceled" before the step's own bound
+    # is ever reached. See github/copilot-sdk#2525.
+    timeout-minutes: 100
     defaults:
       run:
         shell: bash
@@ -206,7 +213,21 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        # TODO: Re-enable Windows after fixing the napi-oop peer shutdown crash.
+        # Windows was previously excluded here because of a napi-oop peer
+        # shutdown crash; the runtime no longer depends on a Node
+        # child/parent process (napi-oop is gone), so that specific failure
+        # mode no longer applies. However, actually running Windows in this
+        # job (github/copilot-sdk#2531) reproduced a *different*, still-open
+        # problem: real native `STATUS_ACCESS_VIOLATION` crashes (SIGSEGV)
+        # partway through the full E2E suite, with no single deterministic
+        # reproducer — consistent with native memory corruption in the
+        # shared runtime cdylib rather than anything fixable from this SDK's
+        # FFI bindings. An identical crash class (AccessViolationException)
+        # was independently reproduced on Windows in-process in the .NET SDK
+        # in the same PR, ruling out a per-language binding bug. Retrying
+        # after rebasing onto CLI 1.0.84-4 reproduced the same blocker.
+        # Tracked upstream at github/copilot-agent-runtime#18990; re-add
+        # windows-latest here once that's resolved. See github/copilot-sdk#2525.
         os: [ubuntu-latest, macos-latest]
     runs-on: ${{ matrix.os }}
     timeout-minutes: 20

dotnet/src/FfiRuntimeHost.cs

@@ -45,9 +45,30 @@ private enum NativeCleanupResult
     /// <summary>Logical name the native interop layer binds the cdylib to.</summary>
     private const string LibraryName = "copilot_runtime";
     private const int CleanupRetryDelayMilliseconds = 100;
+    private static readonly TimeSpan s_hostShutdownTimeout = TimeSpan.FromSeconds(10);
     private static readonly object QuarantineLock = new();
     private static readonly HashSet<FfiRuntimeHost> QuarantinedHosts = [];
 
+    /// <summary>
+    /// Serializes native host lifecycle transitions (<c>host_start</c>/<c>connection_open</c>
+    /// in <see cref="StartAsync"/> against <c>host_shutdown</c> in <see cref="Dispose"/>)
+    /// process-wide.
+    /// </summary>
+    /// <remarks>
+    /// Bounding <see cref="Dispose"/>'s wait (see <see cref="s_hostShutdownTimeout"/>) means a
+    /// slow native shutdown can still be running on an abandoned background thread after
+    /// Dispose() has already returned to its caller. Observed on Windows in-process CI: a new
+    /// client's host_start/connection_open overlapping with a different client's still-draining
+    /// host_shutdown corrupted shared native state and crashed the process with an
+    /// AccessViolationException while writing the new connection's handshake frame (see
+    /// github/copilot-sdk#2525). This gate prevents that overlap: a new Start() waits for any
+    /// in-flight shutdown (abandoned or not) to actually finish before opening a new native
+    /// connection, while multiple already-started hosts remain free to run concurrently (the
+    /// gate is only held during the brief start/open and shutdown transitions, not for the
+    /// lifetime of a live connection).
+    /// </remarks>
+    private static readonly SemaphoreSlim s_nativeLifecycleGate = new(1, 1);
+
     private readonly ILogger _logger;
     private readonly string? _cliEntrypoint;
     private readonly string _libraryPath;
@@ -134,43 +155,54 @@ internal static string GetRuntimeLibraryFileName()
     /// </summary>
     public async Task StartAsync(CancellationToken cancellationToken)
     {
-        // Keep synchronous native startup off the caller's async context.
-        await Task.Run(() =>
+        // See s_nativeLifecycleGate: block a new host_start/connection_open until any
+        // other host's host_shutdown (including one Dispose() already stopped waiting
+        // on) has actually finished.
+        await s_nativeLifecycleGate.WaitAsync(cancellationToken).ConfigureAwait(false);
+        try
         {
-            var argvJson = BuildArgvJson(_cliEntrypoint, _args);
-            var envJson = BuildEnvJson(_environment);
-
-            var serverId = NativeHostStart(argvJson, envJson);
-            if (serverId == 0)
+            // Keep synchronous native startup off the caller's async context.
+            await Task.Run(() =>
             {
-                throw new InvalidOperationException(
-                    $"copilot_runtime_host_start failed (library '{_libraryPath}').");
-            }
+                var argvJson = BuildArgvJson(_cliEntrypoint, _args);
+                var envJson = BuildEnvJson(_environment);
 
-            var connectionId = NativeOpenConnection(serverId);
-            if (connectionId == 0)
-            {
-                _releaseNativeCallback();
-                NativeHostShutdown(serverId);
-                throw new InvalidOperationException("copilot_runtime_connection_open failed.");
-            }
+                var serverId = NativeHostStart(argvJson, envJson);
+                if (serverId == 0)
+                {
+                    throw new InvalidOperationException(
+                        $"copilot_runtime_host_start failed (library '{_libraryPath}').");
+                }
 
-            lock (_lifecycleLock)
-            {
-                _serverId = serverId;
-                _connectionId = connectionId;
-                _sendStream = new CallbackSendStream(SendFrame);
-                if (_disposed)
+                var connectionId = NativeOpenConnection(serverId);
+                if (connectionId == 0)
                 {
-                    if (TryFinalizeNativeCleanup() == NativeCleanupResult.Retry)
+                    _releaseNativeCallback();
+                    NativeHostShutdown(serverId);
+                    throw new InvalidOperationException("copilot_runtime_connection_open failed.");
+                }
+
+                lock (_lifecycleLock)
+                {
+                    _serverId = serverId;
+                    _connectionId = connectionId;
+                    _sendStream = new CallbackSendStream(SendFrame);
+                    if (_disposed)
                     {
-                        ScheduleNativeCleanupRetry();
+                        if (TryFinalizeNativeCleanup() == NativeCleanupResult.Retry)
+                        {
+                            ScheduleNativeCleanupRetry();
+                        }
+                        throw new InvalidOperationException(
+                            "FfiRuntimeHost was disposed during startup.");
                     }
-                    throw new InvalidOperationException(
-                        "FfiRuntimeHost was disposed during startup.");
                 }
-            }
-        }, cancellationToken).ConfigureAwait(false);
+            }, cancellationToken).ConfigureAwait(false);
+        }
+        finally
+        {
+            s_nativeLifecycleGate.Release();
+        }
 
         if (_logger.IsEnabled(LogLevel.Debug))
         {
@@ -306,24 +338,48 @@ private NativeCleanupResult TryFinalizeNativeCleanup()
 
         if (_serverId != 0)
         {
+            var serverId = _serverId;
+            _serverId = 0;
+            ShutdownHost(serverId);
+        }
+
+        return NativeCleanupResult.Complete;
+    }
+
+    private void ShutdownHost(uint serverId)
+    {
+        var shutdownTask = Task.Run(() =>
+        {
+            // See s_nativeLifecycleGate: hold it for the true duration of host_shutdown
+            // (even past the point Dispose() below stops waiting), so a concurrent
+            // StartAsync() on another instance can't overlap host_start/connection_open
+            // with this shutdown still draining.
+            s_nativeLifecycleGate.Wait();
             try
             {
-                if (!_hostShutdown(_serverId) && _logger.IsEnabled(LogLevel.Debug))
+                if (!_hostShutdown(serverId) && _logger.IsEnabled(LogLevel.Debug))
                 {
                     _logger.LogDebug(
                         "FfiRuntimeHost: host_shutdown did not recognize server {ServerId}",
-                        _serverId);
+                        serverId);
                 }
             }
             catch (Exception ex)
             {
                 _logger.LogDebug(ex, "FfiRuntimeHost: host_shutdown failed");
             }
+            finally
+            {
+                s_nativeLifecycleGate.Release();
+            }
+        });
 
-            _serverId = 0;
+        if (!shutdownTask.Wait(s_hostShutdownTimeout))
+        {
+            _logger.LogWarning(
+                "FfiRuntimeHost: host_shutdown did not complete within {Timeout}; abandoning wait.",
+                s_hostShutdownTimeout);
         }
-
-        return NativeCleanupResult.Complete;
     }
 
     private void ScheduleNativeCleanupRetry()

dotnet/test/E2E/ClientE2ETests.cs

@@ -74,6 +74,30 @@ public async Task Should_Force_Stop_Without_Cleanup(bool useStdio)
         await client.ForceStopAsync();
     }
 
+    // Regression coverage for github/copilot-sdk#2525: ForceStopAsync must be a bounded,
+    // immediate hard stop even for the in-process (FFI) host, where there is no child
+    // process to reap if the native runtime's own shutdown path hangs or is slow (e.g.
+    // while closing its SQLite session store). FfiRuntimeHost.Dispose() bounds its wait
+    // on the native copilot_runtime_host_shutdown call so this cannot hang indefinitely;
+    // this test fails fast (via its own generous timeout) instead of hanging the CI job
+    // if that regresses, and its logged elapsed time doubles as shutdown-performance data.
+    [Fact]
+    public async Task Should_Force_Stop_Over_InProcess_Ffi_Within_Bounded_Time()
+    {
+        using var client = new CopilotClient(new CopilotClientOptions
+        {
+            Connection = RuntimeConnection.ForInProcess(),
+        });
+
+        await Ctx.CreateSessionAsync(client, new SessionConfig { OnPermissionRequest = PermissionHandler.ApproveAll });
+
+        var forceStopTask = client.ForceStopAsync();
+        var completed = await Task.WhenAny(forceStopTask, Task.Delay(TimeSpan.FromSeconds(30)));
+
+        Assert.Same(forceStopTask, completed);
+        await forceStopTask;
+    }
+
     [Theory]
     [InlineData(true)]   // stdio transport
     [InlineData(false)]  // TCP transport

go/internal/e2e/inprocess_ffi_e2e_test.go

@@ -2,6 +2,7 @@ package e2e
 
 import (
 	"testing"
+	"time"
 
 	copilot "github.com/github/copilot-sdk/go"
 	"github.com/github/copilot-sdk/go/internal/e2e/testharness"
@@ -59,4 +60,36 @@ func TestInProcessFfiE2E(t *testing.T) {
 			t.Errorf("Expected no errors on stop, got %v", err)
 		}
 	})
+
+	t.Run("should force stop over in-process FFI within a bounded time", func(t *testing.T) {
+		// Regression test for github/copilot-sdk#2525: the in-process FFI
+		// host's Dispose used to call the native host_shutdown export
+		// in-line with no timeout. A slow or stuck native shutdown (observed
+		// on Windows, closing the runtime's SQLite session store) would hang
+		// ForceStop indefinitely, even though ForceStop is documented as the
+		// bounded recovery path for exactly a hung/slow Stop. Asserts that
+		// ForceStop returns within a generous bound instead of hanging.
+		client := copilot.NewClient(&copilot.ClientOptions{
+			Connection: copilot.InProcessConnection{},
+		})
+
+		if err := client.Start(t.Context()); err != nil {
+			t.Fatalf("Failed to start client over in-process FFI: %v", err)
+		}
+		if _, err := client.Ping(t.Context(), "hello before force stop"); err != nil {
+			t.Fatalf("Failed to ping: %v", err)
+		}
+
+		done := make(chan struct{})
+		go func() {
+			client.ForceStop()
+			close(done)
+		}()
+
+		select {
+		case <-done:
+		case <-time.After(20 * time.Second):
+			t.Fatal("ForceStop did not complete within a bounded time")
+		}
+	})
 }