fix: resolve CheckedContinuation deadlock in NetworkTransport on actor dealloc (Swift 6.3)#213
Draft
doozMen wants to merge 3 commits intomodelcontextprotocol:mainfrom
Draft
Conversation
ec383c2 to
843a4ab
Compare
843a4ab to
54c1ac4
Compare
) Replace nonisolated(unsafe) on non-Sendable CheckedContinuation properties with a thread-safe LockedValue wrapper using NSLock. This fixes Swift 6.3-dev build failures without bumping platform minimums (preserves macOS 13+/iOS 16+). Fixes modelcontextprotocol#211 Co-authored-by: Stijn Willems <stijn@promptping.ai>
54c1ac4 to
70ea8de
Compare
Contributor
|
Fixed in #203 and available in the latest v0.12 release. Please check. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #214 —
CheckedContinuationnever resumed inNetworkTransportsend()/sendHeartbeat()/receiveData()when the actor deallocates during an in-flight NWConnection operation. This causes a permanent deadlock.Discovered via Swift 6.3 strict concurrency checking, but the bug exists in all Swift versions using this transport.
Changes
NetworkTransport.swift
send(): Changed completion closure capture from[weak self]to[weak self, continuation]. Addedcontinuation.resume(throwing:)in theguard let self elsebranch.sendHeartbeat(): Same fix — completion closure now capturescontinuationdirectly and resumes with error on dealloc.receiveData(): Changed from implicit strongselfcapture to explicit[weak self, continuation]for consistency. All four branches already resumed the continuation; this hardens the capture semantics.Client.swift
withBatch(): Fixed early-return path to returnresultinstead of implicit void (was broken for non-VoidgenericT).ClientTests.swift
testBatchRequestEmptyNonVoid()— verifieswithBatchreturns the closure result when no requests are added (non-Void path).Housekeeping
.claude/settings.local.jsonand.swift-version.gitignoreThe Pattern
CheckedContinuationisSendable— safe to capture directly alongside[weak self]. This decouples the continuation's lifetime from the actor, guaranteeing resumption even on dealloc.Testing
swift build— cleanswift test --filter testBatchRequestEmpty— 2 tests passsendHeartbeat()— already correct in outer guard, fixed inner completionLoggerisnonisolated let+Sendable— safe to access from completion closures