[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,6 +188,15 @@ 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. 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.

.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,20 @@ 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. 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

@@ -3,6 +3,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 using Microsoft.Extensions.Logging;
+using System.Diagnostics;
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
@@ -38,6 +39,44 @@
     /// <summary>Logical name the native interop layer binds the cdylib to.</summary>
     private const string LibraryName = "copilot_runtime";
 
+    /// <summary>
+    /// Upper bound on how long <see cref="Dispose"/> waits for the native
+    /// <c>copilot_runtime_host_shutdown</c> call to return.
+    /// </summary>
+    /// <remarks>
+    /// This call runs the loaded runtime's own teardown (including closing its SQLite
+    /// session store) synchronously in this process, with no cancellation hook exposed
+    /// across the FFI boundary. A caller may already have asked the runtime to shut down
+    /// gracefully over JSON-RPC (<c>Runtime.ShutdownAsync</c>) before reaching here, so
+    /// this call is expected to be fast; it exists mainly to release the loaded
+    /// library's resources. But because in-process hosting shares this process (there is
+    /// no child process to kill if it does not return), a stuck or slow native shutdown
+    /// would otherwise hang <see cref="Dispose"/> forever, defeating <c>ForceStopAsync</c>'s
+    /// contract of an immediate hard stop. Bounding the wait keeps teardown deterministic
+    /// even if the runtime's shutdown path never returns; see github/copilot-sdk#2525.
+    /// </remarks>
+    private static readonly TimeSpan s_hostShutdownTimeout = TimeSpan.FromSeconds(10);
+
+    /// <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;
@@ -103,30 +142,41 @@
     /// </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);
-
-            _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);
 
-            _connectionId = NativeOpenConnection(_serverId);
-            if (_connectionId == 0)
-            {
-                DisposeNativeCallback();
-                NativeHostShutdown(_serverId);
-                _serverId = 0;
-                throw new InvalidOperationException("copilot_runtime_connection_open failed.");
-            }
+                _serverId = NativeHostStart(argvJson, envJson);
+                if (_serverId == 0)
+                {
+                    throw new InvalidOperationException(
+                        $"copilot_runtime_host_start failed (library '{_libraryPath}').");
+                }
 
-            _sendStream = new CallbackSendStream(SendFrame);
-        }, cancellationToken).ConfigureAwait(false);
+                _connectionId = NativeOpenConnection(_serverId);
+                if (_connectionId == 0)
+                {
+                    DisposeNativeCallback();
+                    NativeHostShutdown(_serverId);
+                    _serverId = 0;
+                    throw new InvalidOperationException("copilot_runtime_connection_open failed.");
+                }
+
+                _sendStream = new CallbackSendStream(SendFrame);
+            }, cancellationToken).ConfigureAwait(false);
+        }
+        finally
+        {
+            s_nativeLifecycleGate.Release();
+        }
 
         if (_logger.IsEnabled(LogLevel.Debug))
         {
@@ -225,21 +275,64 @@
             _logger.LogDebug(ex, "FfiRuntimeHost: connection_close failed");
         }
 
-        try
+        _receiveStream.Complete();
+
+        var serverId = _serverId;
+        _serverId = 0;
+        if (serverId == 0)
+        {
+            DisposeNativeCallback();
+            return;
+        }
+
+        var shutdownTimestamp = Stopwatch.GetTimestamp();
+
+        // Run the blocking native call on a pooled thread so this Dispose() call can
+        // enforce a bound on it instead of hanging indefinitely if the runtime's own
+        // shutdown never returns. The callback GCHandle is freed only once the native
+        // call actually returns (inside the task, not here), so a slow-but-eventually-
+        // completing shutdown cannot race a native callback against a freed handle even
+        // when this method stops waiting early.
+        var shutdownTask = Task.Run(() =>
         {
-            if (_serverId != 0)
+            // 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
             {
-                NativeHostShutdown(_serverId);
-                _serverId = 0;
+                NativeHostShutdown(serverId);
             }
+            catch (Exception ex)
+            {
+                _logger.LogDebug(ex, "FfiRuntimeHost: host_shutdown failed");
+            }
+            finally
+            {
+                DisposeNativeCallback();
+                s_nativeLifecycleGate.Release();
+            }
+        });
+
+        if (shutdownTask.Wait(s_hostShutdownTimeout))
+        {
+            LoggingHelpers.LogTiming(_logger, LogLevel.Debug, null,
+                "FfiRuntimeHost: host_shutdown complete. Elapsed={Elapsed}",
+                shutdownTimestamp);
         }
-        catch (Exception ex)
+        else
         {
-            _logger.LogDebug(ex, "FfiRuntimeHost: host_shutdown failed");
+            // The native call (and the callback cleanup that follows it) keeps running
+            // on the abandoned background thread; we just stop waiting on it here so the
+            // caller (e.g. ForceStopAsync) is not blocked forever. This should be rare
+            // and indicates a runtime-side shutdown defect worth reporting upstream, not
+            // something for the SDK to retry.
+            LoggingHelpers.LogTiming(_logger, LogLevel.Warning, null,
+                "FfiRuntimeHost: host_shutdown did not complete within Elapsed={Elapsed}, Timeout={Timeout}; abandoning wait.",
+                shutdownTimestamp,
+                s_hostShutdownTimeout);
         }
-
-        _receiveStream.Complete();
-        DisposeNativeCallback();
     }
 
     /// <summary>Length as the native pointer-sized unsigned integer the ABI expects.</summary>

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")
+		}
+	})
 }