Skip to content

Commit 6892034

Browse files
committed
Prevent double continuation resumption in client
Consolidate request removal and continuation resumption logic to ensure each request's continuation is resumed exactly once, preventing "SWIFT TASK CONTINUATION MISUSE" errors during network failures.
1 parent 87f33d0 commit 6892034

File tree

1 file changed

+40
-19
lines changed

1 file changed

+40
-19
lines changed

Sources/MCP/Client/Client.swift

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -267,12 +267,12 @@ public actor Client {
267267
throw MCPError.internalError("Client connection not initialized")
268268
}
269269

270-
// Use the actor's encoder
271270
let requestData = try encoder.encode(request)
272271

273272
// Store the pending request first
274273
return try await withCheckedThrowingContinuation { continuation in
275274
Task {
275+
// Add the pending request before attempting to send
276276
self.addPendingRequest(
277277
id: request.id,
278278
continuation: continuation,
@@ -284,9 +284,15 @@ public actor Client {
284284
// Use the existing connection send
285285
try await connection.send(requestData)
286286
} catch {
287-
// If send fails immediately, resume continuation and remove pending request
288-
continuation.resume(throwing: error)
289-
self.removePendingRequest(id: request.id) // Ensure cleanup on send error
287+
// If send fails, try to remove the pending request.
288+
// Resume with the send error only if we successfully removed the request,
289+
// indicating the response handler hasn't processed it yet.
290+
if self.removePendingRequest(id: request.id) != nil {
291+
continuation.resume(throwing: error)
292+
}
293+
// Otherwise, the request was already removed by the response handler
294+
// or by disconnect, so the continuation was already resumed.
295+
// Do nothing here.
290296
}
291297
}
292298
}
@@ -300,8 +306,8 @@ public actor Client {
300306
pendingRequests[id] = AnyPendingRequest(PendingRequest(continuation: continuation))
301307
}
302308

303-
private func removePendingRequest(id: ID) {
304-
pendingRequests.removeValue(forKey: id)
309+
private func removePendingRequest(id: ID) -> AnyPendingRequest? {
310+
return pendingRequests.removeValue(forKey: id)
305311
}
306312

307313
// MARK: - Batching
@@ -562,14 +568,23 @@ public actor Client {
562568
"Processing response",
563569
metadata: ["id": "\(response.id)"])
564570

565-
switch response.result {
566-
case .success(let value):
567-
request.resume(returning: value)
568-
case .failure(let error):
569-
request.resume(throwing: error)
571+
// Attempt to remove the pending request.
572+
// Resume with the response only if it hadn't yet been removed.
573+
if self.removePendingRequest(id: response.id) != nil {
574+
switch response.result {
575+
case .success(let value):
576+
request.resume(returning: value)
577+
case .failure(let error):
578+
request.resume(throwing: error)
579+
}
580+
} else {
581+
// Request was already removed
582+
// (e.g., by send error handler or disconnect).
583+
await logger?.warning(
584+
"Attempted to handle response for already removed request",
585+
metadata: ["id": "\(response.id)"]
586+
)
570587
}
571-
572-
removePendingRequest(id: response.id)
573588
}
574589

575590
private func handleMessage(_ message: Message<AnyNotification>) async {
@@ -619,14 +634,20 @@ public actor Client {
619634
private func handleBatchResponse(_ responses: [AnyResponse]) async {
620635
await logger?.debug("Processing batch response", metadata: ["count": "\(responses.count)"])
621636
for response in responses {
622-
// Look up the pending request for this specific ID within the batch
623-
if let request = pendingRequests[response.id] {
624-
// Reuse the existing single response handler logic
625-
await handleResponse(response, for: request)
637+
// Attempt to remove the pending request. If successful, removedRequestBox will contain the request.
638+
if let removedRequestBox = self.removePendingRequest(id: response.id) {
639+
// If we successfully removed it, handle the response using the removed request box.
640+
switch response.result {
641+
case .success(let value):
642+
removedRequestBox.resume(returning: value)
643+
case .failure(let error):
644+
removedRequestBox.resume(throwing: error)
645+
}
626646
} else {
627-
// Log if a response ID doesn't match any pending request
647+
// If removal failed, it means the request ID was not found (or already handled).
648+
// Log a warning.
628649
await logger?.warning(
629-
"Received response in batch for unknown request ID",
650+
"Received response in batch for unknown or already handled request ID",
630651
metadata: ["id": "\(response.id)"]
631652
)
632653
}

0 commit comments

Comments
 (0)