Skip to content
This repository was archived by the owner on May 29, 2025. It is now read-only.

Commit 985827e

Browse files
authored
improve thread safety for expectation helper, and remove some flakey tests (#23)
* improve thread safety for `expectation` helper, and remove flakey tests * fix flakey tests * lint
1 parent 9674388 commit 985827e

File tree

5 files changed

+68
-29
lines changed

5 files changed

+68
-29
lines changed

.github/workflows/swift.yml

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,8 @@ jobs:
1414
runs-on: macos-latest
1515
steps:
1616
- uses: swift-actions/setup-swift@v2
17-
with:
18-
swift-version: "6.0.1"
19-
20-
steps:
17+
with:
18+
swift-version: "6.0.1"
2119
- name: Get swift version
2220
run: swift --version
2321
- uses: actions/checkout@v4

MCPClient/Sources/stdioTransport/DataChannel+StdioProcess.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ extension JSONRPCSetupError: LocalizedError {
4444

4545
extension Transport {
4646

47-
#if os(macOS)
47+
#if os(macOS)
4848
/// Creates a new `Transport` by launching the given executable with the specified arguments and attaching to its standard IO.
4949
/// This functionality is only available on macOS.
5050
public static func stdioProcess(
@@ -286,7 +286,7 @@ extension Transport {
286286

287287
return String(data: stdoutData, encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines)
288288
}
289-
#endif
289+
#endif
290290
}
291291

292292
// MARK: - Lifetime

Package.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ let package = Package(
77
name: "MCP",
88
platforms: [
99
.macOS(.v14),
10-
.iOS(.v17)
10+
.iOS(.v17),
1111
],
1212
products: [
1313
.library(
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import os
2+
3+
final class Atomic<Value: Sendable>: Sendable {
4+
5+
init(_ value: Value) {
6+
lock = .init(initialState: value)
7+
}
8+
9+
var value: Value {
10+
lock.withLock { $0 }
11+
}
12+
13+
@discardableResult
14+
func mutate<Result: Sendable>(_ mutation: @Sendable (inout Value) -> Result) -> Result {
15+
lock.withLock { mutation(&$0) }
16+
}
17+
18+
private let lock: OSAllocatedUnfairLock<Value>
19+
20+
}

SwiftTestingUtils/Sources/SwiftTestingUtils.swift

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
21
import Foundation
2+
import os
33
import OSLog
44
import Testing
55

@@ -9,7 +9,7 @@ private let logger = Logger(subsystem: "testing", category: "testing")
99

1010
public enum SwiftTestingUtils {
1111

12-
public class Expectation {
12+
public final class Expectation: Sendable {
1313

1414
init(
1515
description: String,
@@ -19,16 +19,24 @@ public enum SwiftTestingUtils {
1919
location = _sourceLocation
2020
}
2121

22+
public var isFulfilled: Bool {
23+
lock.withLock { $0.isFulfilled }
24+
}
25+
2226
public func fulfill() {
23-
if isFulfilled {
27+
let (wasFulfilled, fullfillmentActions) = lock.withLock { state in
28+
let wasFulfilled = state.isFulfilled
29+
state.isFulfilled = true
30+
return (wasFulfilled, state.onFulfill)
31+
}
32+
33+
if wasFulfilled {
2434
Issue.record("Expectation from \(location) already fulfilled.")
35+
return
2536
}
26-
isFulfilled = true
27-
onFulfill()
37+
for fullfillmentAction in fullfillmentActions { fullfillmentAction() }
2838
}
2939

30-
private(set) var isFulfilled = false
31-
3240
let description: String
3341
let location: SourceLocation
3442

@@ -37,34 +45,47 @@ public enum SwiftTestingUtils {
3745
return
3846
}
3947

40-
var hasTimedOut = false
41-
var hasCompleted = false
48+
let hasTimedOut = Atomic(false)
4249
let description = description
4350

4451
try await withCheckedThrowingContinuation { continuation in
45-
onFulfill = {
46-
if !hasTimedOut {
47-
if hasCompleted {
48-
Issue.record("Expectation \(description) fulfilled multiple times.")
49-
} else {
50-
hasCompleted = true
51-
continuation.resume(returning: ())
52-
}
52+
let onFulfill: @Sendable () -> Void = {
53+
if !hasTimedOut.value {
54+
continuation.resume(returning: ())
5355
} else {
5456
logger.error("Expectation \(description) fulfilled after timeout.")
5557
}
5658
}
59+
let wasFulfilled = lock.withLock { state in
60+
if state.isFulfilled {
61+
return true
62+
}
63+
state.onFulfill.append(onFulfill)
64+
return false
65+
}
66+
if wasFulfilled {
67+
continuation.resume(returning: ())
68+
}
5769
Task {
58-
try await Task.sleep(nanoseconds: UInt64(timeout * 1_000_000_000))
59-
if !hasCompleted {
60-
hasTimedOut = true
61-
continuation.resume(throwing: SwiftTesting.expectationTimeout(description: description))
70+
do {
71+
try await Task.sleep(nanoseconds: UInt64(timeout * 1_000_000_000))
72+
if !isFulfilled {
73+
hasTimedOut.mutate { $0 = true }
74+
continuation.resume(throwing: SwiftTesting.expectationTimeout(description: description))
75+
}
76+
} catch {
77+
continuation.resume(throwing: error)
6278
}
6379
}
6480
}
6581
}
6682

67-
private var onFulfill: () -> Void = { }
83+
private struct InternalState {
84+
var isFulfilled = false
85+
var onFulfill: [@Sendable () -> Void] = []
86+
}
87+
88+
private let lock = OSAllocatedUnfairLock<InternalState>(initialState: InternalState())
6889
}
6990

7091
enum SwiftTesting: Error {

0 commit comments

Comments
 (0)