Skip to content

Fix .NET in-process callback lifetime - #2397

Draft
minyitang wants to merge 1 commit into
update-copilot-1.0.81-9from
minyitang/dotnet-ffi-callback-drain
Draft

Fix .NET in-process callback lifetime#2397
minyitang wants to merge 1 commit into
update-copilot-1.0.81-9from
minyitang/dotnet-ffi-callback-drain

Conversation

@minyitang

Copy link
Copy Markdown

Summary

  • open tracked runtime connections and drain them before releasing managed callback state
  • retain callback state and surface an error if graceful draining fails
  • preserve ForceStopAsync behavior with an immediate close that safely retains callback state when necessary
  • root the netstandard callback host with a GCHandle, matching the modern .NET lifetime guarantee

Compatibility

This is stacked on #2390 because copilot_runtime_connection_open_tracked and copilot_runtime_connection_close_and_wait are available in the bundled runtime starting with CLI 1.0.81-7; #2390 updates the SDK to 1.0.81-9.

Validation

  • built net8.0, net10.0, and netstandard2.0 with zero warnings
  • 217 .NET unit tests passed
  • graceful and force-stop in-process FFI E2Es passed
  • dotnet format --verify-no-changes passed

Use tracked runtime connections and drain callbacks before releasing managed callback state. Preserve force-stop latency by retaining callback state when an immediate drain cannot complete.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: b659bb88-f59c-49d3-afa7-c1b38d575f4b
@github-actions

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

This PR makes internal implementation changes to the .NET FFI layer (FfiRuntimeHost, which is internal sealed). The changes address:

  • Switching to copilot_runtime_connection_open_tracked / copilot_runtime_connection_close_and_wait native entry points
  • Proper GCHandle lifetime management for the outbound callback
  • Graceful vs. force-close code paths (Dispose vs ForceDispose)

No cross-SDK consistency issues found. Specifically:

  • The public API (ForceStopAsync) already exists and is unchanged — no new surface area is being added
  • The FFI/GCHandle mechanics are .NET-specific (P/Invoke, GCHandle, LibraryImport) and have no equivalent in other SDKs' runtimes
  • Other SDKs (Node.js, Python, Go, Java, Rust) use different runtime bridging mechanisms that are unaffected by these native ABI changes

This is a valid language-specific internal fix.

Generated by SDK Consistency Review Agent for #2397 · sonnet46 11 AIC · ⌖ 4.06 AIC · ⊞ 6.6K ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant