Skip to content

Commit 4de7a26

Browse files
authoredApr 29, 2025··
Fix a number of sendable warnings in test utilities (#836)
Motivation: The test utilities have a number of sendability issues. Modifications: - Fix the issues Result: Fewer warnings
1 parent a4fcd70 commit 4de7a26

File tree

1 file changed

+119
-90
lines changed

1 file changed

+119
-90
lines changed
 

‎Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift

Lines changed: 119 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -95,15 +95,13 @@ func withCLocaleSetToGerman(_ body: () throws -> Void) throws {
9595
try body()
9696
}
9797

98-
class TestHTTPDelegate: HTTPClientResponseDelegate {
98+
final class TestHTTPDelegate: HTTPClientResponseDelegate {
9999
typealias Response = Void
100100

101101
init(backpressureEventLoop: EventLoop? = nil) {
102-
self.backpressureEventLoop = backpressureEventLoop
102+
self.state = NIOLockedValueBox(MutableState(backpressureEventLoop: backpressureEventLoop))
103103
}
104104

105-
var backpressureEventLoop: EventLoop?
106-
107105
enum State {
108106
case idle
109107
case head(HTTPResponseHead)
@@ -112,77 +110,96 @@ class TestHTTPDelegate: HTTPClientResponseDelegate {
112110
case error(Error)
113111
}
114112

115-
var state = State.idle
113+
struct MutableState: Sendable {
114+
var state: State = .idle
115+
var backpressureEventLoop: EventLoop?
116+
}
117+
118+
let state: NIOLockedValueBox<MutableState>
116119

117120
func didReceiveHead(task: HTTPClient.Task<Response>, _ head: HTTPResponseHead) -> EventLoopFuture<Void> {
118-
self.state = .head(head)
119-
return (self.backpressureEventLoop ?? task.eventLoop).makeSucceededFuture(())
121+
let eventLoop = self.state.withLockedValue {
122+
$0.state = .head(head)
123+
return ($0.backpressureEventLoop ?? task.eventLoop)
124+
}
125+
126+
return eventLoop.makeSucceededVoidFuture()
120127
}
121128

122129
func didReceiveBodyPart(task: HTTPClient.Task<Response>, _ buffer: ByteBuffer) -> EventLoopFuture<Void> {
123-
switch self.state {
124-
case .head(let head):
125-
self.state = .body(head, buffer)
126-
case .body(let head, var body):
127-
var buffer = buffer
128-
body.writeBuffer(&buffer)
129-
self.state = .body(head, body)
130-
default:
131-
preconditionFailure("expecting head or body")
130+
let eventLoop = self.state.withLockedValue {
131+
switch $0.state {
132+
case .head(let head):
133+
$0.state = .body(head, buffer)
134+
case .body(let head, var body):
135+
var buffer = buffer
136+
body.writeBuffer(&buffer)
137+
$0.state = .body(head, body)
138+
default:
139+
preconditionFailure("expecting head or body")
140+
}
141+
return ($0.backpressureEventLoop ?? task.eventLoop)
132142
}
133-
return (self.backpressureEventLoop ?? task.eventLoop).makeSucceededFuture(())
143+
144+
return eventLoop.makeSucceededVoidFuture()
134145
}
135146

136147
func didFinishRequest(task: HTTPClient.Task<Response>) throws {}
137148
}
138149

139-
class CountingDelegate: HTTPClientResponseDelegate {
150+
final class CountingDelegate: HTTPClientResponseDelegate {
140151
typealias Response = Int
141152

142-
var count = 0
153+
private let _count = NIOLockedValueBox(0)
143154

144155
func didReceiveBodyPart(task: HTTPClient.Task<Response>, _ buffer: ByteBuffer) -> EventLoopFuture<Void> {
145156
let str = buffer.getString(at: 0, length: buffer.readableBytes)
146157
if str?.starts(with: "id:") ?? false {
147-
self.count += 1
158+
self._count.withLockedValue { $0 += 1 }
148159
}
149160
return task.eventLoop.makeSucceededFuture(())
150161
}
151162

152163
func didFinishRequest(task: HTTPClient.Task<Response>) throws -> Int {
153-
self.count
164+
self._count.withLockedValue { $0 }
154165
}
155166
}
156167

157-
class DelayOnHeadDelegate: HTTPClientResponseDelegate {
168+
final class DelayOnHeadDelegate: HTTPClientResponseDelegate {
158169
typealias Response = ByteBuffer
159170

160171
let eventLoop: EventLoop
161-
let didReceiveHead: (HTTPResponseHead, EventLoopPromise<Void>) -> Void
162-
163-
private var data: ByteBuffer
172+
let didReceiveHead: @Sendable (HTTPResponseHead, EventLoopPromise<Void>) -> Void
164173

165-
private var mayReceiveData = false
174+
struct State: Sendable {
175+
var data: ByteBuffer
176+
var mayReceiveData = false
177+
var expectError = false
178+
}
166179

167-
private var expectError = false
180+
private let state: NIOLockedValueBox<State>
168181

169-
init(eventLoop: EventLoop, didReceiveHead: @escaping (HTTPResponseHead, EventLoopPromise<Void>) -> Void) {
182+
init(eventLoop: EventLoop, didReceiveHead: @escaping @Sendable (HTTPResponseHead, EventLoopPromise<Void>) -> Void) {
170183
self.eventLoop = eventLoop
171184
self.didReceiveHead = didReceiveHead
172-
self.data = ByteBuffer()
185+
self.state = NIOLockedValueBox(State(data: ByteBuffer()))
173186
}
174187

175188
func didReceiveHead(task: HTTPClient.Task<Response>, _ head: HTTPResponseHead) -> EventLoopFuture<Void> {
176-
XCTAssertFalse(self.mayReceiveData)
177-
XCTAssertFalse(self.expectError)
189+
self.state.withLockedValue {
190+
XCTAssertFalse($0.mayReceiveData)
191+
XCTAssertFalse($0.expectError)
192+
}
178193

179194
let promise = self.eventLoop.makePromise(of: Void.self)
180-
promise.futureResult.whenComplete {
181-
switch $0 {
182-
case .success:
183-
self.mayReceiveData = true
184-
case .failure:
185-
self.expectError = true
195+
promise.futureResult.whenComplete { result in
196+
self.state.withLockedValue { state in
197+
switch result {
198+
case .success:
199+
state.mayReceiveData = true
200+
case .failure:
201+
state.expectError = true
202+
}
186203
}
187204
}
188205

@@ -191,20 +208,26 @@ class DelayOnHeadDelegate: HTTPClientResponseDelegate {
191208
}
192209

193210
func didReceiveBodyPart(task: HTTPClient.Task<Response>, _ buffer: ByteBuffer) -> EventLoopFuture<Void> {
194-
XCTAssertTrue(self.mayReceiveData)
195-
XCTAssertFalse(self.expectError)
196-
self.data.writeImmutableBuffer(buffer)
211+
self.state.withLockedValue {
212+
XCTAssertTrue($0.mayReceiveData)
213+
XCTAssertFalse($0.expectError)
214+
$0.data.writeImmutableBuffer(buffer)
215+
}
197216
return self.eventLoop.makeSucceededFuture(())
198217
}
199218

200219
func didFinishRequest(task: HTTPClient.Task<Response>) throws -> Response {
201-
XCTAssertTrue(self.mayReceiveData)
202-
XCTAssertFalse(self.expectError)
203-
return self.data
220+
self.state.withLockedValue {
221+
XCTAssertTrue($0.mayReceiveData)
222+
XCTAssertFalse($0.expectError)
223+
return $0.data
224+
}
204225
}
205226

206227
func didReceiveError(task: HTTPClient.Task<ByteBuffer>, _ error: Error) {
207-
XCTAssertTrue(self.expectError)
228+
self.state.withLockedValue {
229+
XCTAssertTrue($0.expectError)
230+
}
208231
}
209232
}
210233

@@ -336,7 +359,7 @@ enum TestTLS {
336359
)
337360
}
338361

339-
internal final class HTTPBin<RequestHandler: ChannelInboundHandler>
362+
internal final class HTTPBin<RequestHandler: ChannelInboundHandler>: Sendable
340363
where
341364
RequestHandler.InboundIn == HTTPServerRequestPart,
342365
RequestHandler.OutboundOut == HTTPServerResponsePart
@@ -415,11 +438,15 @@ where
415438
}
416439

417440
var port: Int {
418-
Int(self.serverChannel.localAddress!.port!)
441+
self.serverChannel.withLockedValue {
442+
Int($0!.localAddress!.port!)
443+
}
419444
}
420445

421446
var socketAddress: SocketAddress {
422-
self.serverChannel.localAddress!
447+
self.serverChannel.withLockedValue {
448+
$0!.localAddress!
449+
}
423450
}
424451

425452
var baseURL: String {
@@ -447,17 +474,17 @@ where
447474

448475
private let mode: Mode
449476
private let sslContext: NIOSSLContext?
450-
private var serverChannel: Channel!
477+
private let serverChannel = NIOLockedValueBox<Channel?>(nil)
451478
private let isShutdown = ManagedAtomic(false)
452-
private let handlerFactory: (Int) -> (RequestHandler)
479+
private let handlerFactory: @Sendable (Int) -> (RequestHandler)
453480

454481
init(
455482
_ mode: Mode = .http1_1(ssl: false, compress: false),
456483
proxy: Proxy = .none,
457484
bindTarget: BindTarget = .localhostIPv4RandomPort,
458485
reusePort: Bool = false,
459486
trafficShapingTargetBytesPerSecond: Int? = nil,
460-
handlerFactory: @escaping (Int) -> (RequestHandler)
487+
handlerFactory: @escaping @Sendable (Int) -> (RequestHandler)
461488
) {
462489
self.mode = mode
463490
self.sslContext = HTTPBin.sslContext(for: mode)
@@ -477,14 +504,14 @@ where
477504

478505
let connectionIDAtomic = ManagedAtomic(0)
479506

480-
self.serverChannel = try! ServerBootstrap(group: self.group)
507+
let serverChannel = try! ServerBootstrap(group: self.group)
481508
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
482509
.serverChannelOption(
483510
ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEPORT),
484511
value: reusePort ? 1 : 0
485512
)
486-
.serverChannelInitializer { channel in
487-
channel.pipeline.addHandler(self.activeConnCounterHandler)
513+
.serverChannelInitializer { [activeConnCounterHandler] channel in
514+
channel.pipeline.addHandler(activeConnCounterHandler)
488515
}.childChannelInitializer { channel in
489516
if let trafficShapingTargetBytesPerSecond = trafficShapingTargetBytesPerSecond {
490517
try! channel.pipeline.syncOperations.addHandler(
@@ -528,6 +555,7 @@ where
528555
return channel.eventLoop.makeFailedFuture(error)
529556
}
530557
}.bind(to: socketAddress).wait()
558+
self.serverChannel.withLockedValue { $0 = serverChannel }
531559
}
532560

533561
private func syncAddHTTPProxyHandlers(
@@ -1092,13 +1120,13 @@ internal final class HTTPBinHandler: ChannelInboundHandler {
10921120
)
10931121
context.write(wrapOutboundOut(.body(.byteBuffer(responseBody))), promise: nil)
10941122
}
1095-
context.eventLoop.scheduleTask(in: self.delay) {
1123+
context.eventLoop.assumeIsolated().scheduleTask(in: self.delay) {
10961124
guard context.channel.isActive else {
10971125
context.close(promise: nil)
10981126
return
10991127
}
11001128

1101-
context.writeAndFlush(self.wrapOutboundOut(.end(nil))).whenComplete { result in
1129+
context.writeAndFlush(self.wrapOutboundOut(.end(nil))).assumeIsolated().whenComplete { result in
11021130
self.isServingRequest = false
11031131
switch result {
11041132
case .success:
@@ -1133,7 +1161,7 @@ internal final class HTTPBinHandler: ChannelInboundHandler {
11331161
}
11341162
}
11351163

1136-
final class ConnectionsCountHandler: ChannelInboundHandler {
1164+
final class ConnectionsCountHandler: ChannelInboundHandler, Sendable {
11371165
typealias InboundIn = Channel
11381166

11391167
private let activeConns = ManagedAtomic(0)
@@ -1152,8 +1180,8 @@ final class ConnectionsCountHandler: ChannelInboundHandler {
11521180

11531181
_ = self.activeConns.loadThenWrappingIncrement(ordering: .relaxed)
11541182
_ = self.createdConns.loadThenWrappingIncrement(ordering: .relaxed)
1155-
channel.closeFuture.whenComplete { _ in
1156-
_ = self.activeConns.loadThenWrappingDecrement(ordering: .relaxed)
1183+
channel.closeFuture.whenComplete { [activeConns] _ in
1184+
_ = activeConns.loadThenWrappingDecrement(ordering: .relaxed)
11571185
}
11581186

11591187
context.fireChannelRead(data)
@@ -1173,7 +1201,7 @@ internal final class CloseWithoutClosingServerHandler: ChannelInboundHandler {
11731201

11741202
func handlerAdded(context: ChannelHandlerContext) {
11751203
self.onClosePromise = context.eventLoop.makePromise()
1176-
self.onClosePromise!.futureResult.whenSuccess(self.callback!)
1204+
self.onClosePromise!.futureResult.assumeIsolated().whenSuccess(self.callback!)
11771205
self.callback = nil
11781206
}
11791207

@@ -1235,7 +1263,7 @@ final class ExpectClosureServerHandler: ChannelInboundHandler {
12351263

12361264
struct EventLoopFutureTimeoutError: Error {}
12371265

1238-
extension EventLoopFuture {
1266+
extension EventLoopFuture where Value: Sendable {
12391267
func timeout(after failDelay: TimeAmount) -> EventLoopFuture<Value> {
12401268
let promise = self.eventLoop.makePromise(of: Value.self)
12411269

@@ -1261,28 +1289,27 @@ struct CollectEverythingLogHandler: LogHandler {
12611289
var logLevel: Logger.Level = .info
12621290
let logStore: LogStore
12631291

1264-
class LogStore {
1292+
final class LogStore: Sendable {
12651293
struct Entry {
12661294
var level: Logger.Level
12671295
var message: String
12681296
var metadata: [String: String]
12691297
}
12701298

1271-
var lock = NIOLock()
1272-
var logs: [Entry] = []
1299+
private let logs = NIOLockedValueBox<[Entry]>([])
12731300

12741301
var allEntries: [Entry] {
12751302
get {
1276-
self.lock.withLock { self.logs }
1303+
self.logs.withLockedValue { $0 }
12771304
}
12781305
set {
1279-
self.lock.withLock { self.logs = newValue }
1306+
self.logs.withLockedValue { $0 = newValue }
12801307
}
12811308
}
12821309

12831310
func append(level: Logger.Level, message: Logger.Message, metadata: Logger.Metadata?) {
1284-
self.lock.withLock {
1285-
self.logs.append(
1311+
self.logs.withLockedValue {
1312+
$0.append(
12861313
Entry(
12871314
level: level,
12881315
message: message.description,
@@ -1301,6 +1328,7 @@ struct CollectEverythingLogHandler: LogHandler {
13011328
level: Logger.Level,
13021329
message: Logger.Message,
13031330
metadata: Logger.Metadata?,
1331+
source: String,
13041332
file: String,
13051333
function: String,
13061334
line: UInt
@@ -1322,10 +1350,10 @@ struct CollectEverythingLogHandler: LogHandler {
13221350
/// consume the bytes by calling ``next()`` on the delegate.
13231351
///
13241352
/// The sole purpose of this class is to enable straight-line stream tests.
1325-
class ResponseStreamDelegate: HTTPClientResponseDelegate {
1353+
final class ResponseStreamDelegate: HTTPClientResponseDelegate {
13261354
typealias Response = Void
13271355

1328-
enum State {
1356+
enum State: Sendable {
13291357
/// The delegate is in the idle state. There are no http response parts to be buffered
13301358
/// and the consumer did not signal a demand. Transitions to all other states are allowed.
13311359
case idle
@@ -1343,10 +1371,11 @@ class ResponseStreamDelegate: HTTPClientResponseDelegate {
13431371
}
13441372

13451373
let eventLoop: EventLoop
1346-
private var state: State = .idle
1374+
private let state: NIOLoopBoundBox<State>
13471375

13481376
init(eventLoop: EventLoop) {
13491377
self.eventLoop = eventLoop
1378+
self.state = .makeBoxSendingValue(.idle, eventLoop: eventLoop)
13501379
}
13511380

13521381
func next() -> EventLoopFuture<ByteBuffer?> {
@@ -1360,25 +1389,25 @@ class ResponseStreamDelegate: HTTPClientResponseDelegate {
13601389
}
13611390

13621391
private func next0() -> EventLoopFuture<ByteBuffer?> {
1363-
switch self.state {
1392+
switch self.state.value {
13641393
case .idle:
13651394
let promise = self.eventLoop.makePromise(of: ByteBuffer?.self)
1366-
self.state = .waitingForBytes(promise)
1395+
self.state.value = .waitingForBytes(promise)
13671396
return promise.futureResult
13681397

13691398
case .buffering(let byteBuffer, done: false):
1370-
self.state = .idle
1399+
self.state.value = .idle
13711400
return self.eventLoop.makeSucceededFuture(byteBuffer)
13721401

13731402
case .buffering(let byteBuffer, done: true):
1374-
self.state = .finished
1403+
self.state.value = .finished
13751404
return self.eventLoop.makeSucceededFuture(byteBuffer)
13761405

13771406
case .waitingForBytes:
13781407
preconditionFailure("Don't call `.next` twice")
13791408

13801409
case .failed(let error):
1381-
self.state = .finished
1410+
self.state.value = .finished
13821411
return self.eventLoop.makeFailedFuture(error)
13831412

13841413
case .finished:
@@ -1408,16 +1437,16 @@ class ResponseStreamDelegate: HTTPClientResponseDelegate {
14081437
func didReceiveBodyPart(task: HTTPClient.Task<Response>, _ buffer: ByteBuffer) -> EventLoopFuture<Void> {
14091438
self.eventLoop.preconditionInEventLoop()
14101439

1411-
switch self.state {
1440+
switch self.state.value {
14121441
case .idle:
1413-
self.state = .buffering(buffer, done: false)
1442+
self.state.value = .buffering(buffer, done: false)
14141443
case .waitingForBytes(let promise):
1415-
self.state = .idle
1444+
self.state.value = .idle
14161445
promise.succeed(buffer)
14171446
case .buffering(var byteBuffer, done: false):
14181447
var buffer = buffer
14191448
byteBuffer.writeBuffer(&buffer)
1420-
self.state = .buffering(byteBuffer, done: false)
1449+
self.state.value = .buffering(byteBuffer, done: false)
14211450
case .buffering(_, done: true), .finished, .failed:
14221451
preconditionFailure("Invalid state: \(self.state)")
14231452
}
@@ -1428,14 +1457,14 @@ class ResponseStreamDelegate: HTTPClientResponseDelegate {
14281457
func didReceiveError(task: HTTPClient.Task<Response>, _ error: Error) {
14291458
self.eventLoop.preconditionInEventLoop()
14301459

1431-
switch self.state {
1460+
switch self.state.value {
14321461
case .idle:
1433-
self.state = .failed(error)
1462+
self.state.value = .failed(error)
14341463
case .waitingForBytes(let promise):
1435-
self.state = .finished
1464+
self.state.value = .finished
14361465
promise.fail(error)
14371466
case .buffering(_, done: false):
1438-
self.state = .failed(error)
1467+
self.state.value = .failed(error)
14391468
case .buffering(_, done: true), .finished, .failed:
14401469
preconditionFailure("Invalid state: \(self.state)")
14411470
}
@@ -1444,14 +1473,14 @@ class ResponseStreamDelegate: HTTPClientResponseDelegate {
14441473
func didFinishRequest(task: HTTPClient.Task<Response>) throws {
14451474
self.eventLoop.preconditionInEventLoop()
14461475

1447-
switch self.state {
1476+
switch self.state.value {
14481477
case .idle:
1449-
self.state = .finished
1478+
self.state.value = .finished
14501479
case .waitingForBytes(let promise):
1451-
self.state = .finished
1480+
self.state.value = .finished
14521481
promise.succeed(nil)
14531482
case .buffering(let byteBuffer, done: false):
1454-
self.state = .buffering(byteBuffer, done: true)
1483+
self.state.value = .buffering(byteBuffer, done: true)
14551484
case .buffering(_, done: true), .finished, .failed:
14561485
preconditionFailure("Invalid state: \(self.state)")
14571486
}
@@ -1473,7 +1502,7 @@ class HTTPEchoHandler: ChannelInboundHandler {
14731502
case .body(let bytes):
14741503
context.writeAndFlush(self.wrapOutboundOut(.body(.byteBuffer(bytes))), promise: nil)
14751504
case .end:
1476-
context.writeAndFlush(self.wrapOutboundOut(.end(nil))).whenSuccess {
1505+
context.writeAndFlush(self.wrapOutboundOut(.end(nil))).assumeIsolated().whenSuccess {
14771506
context.close(promise: nil)
14781507
}
14791508
}
@@ -1495,7 +1524,7 @@ final class HTTPEchoHeaders: ChannelInboundHandler {
14951524
case .body:
14961525
break
14971526
case .end:
1498-
context.writeAndFlush(self.wrapOutboundOut(.end(nil))).whenSuccess {
1527+
context.writeAndFlush(self.wrapOutboundOut(.end(nil))).assumeIsolated().whenSuccess {
14991528
context.close(promise: nil)
15001529
}
15011530
}
@@ -1661,7 +1690,7 @@ final class BasicInboundTrafficShapingHandler: ChannelDuplexHandler {
16611690
let buffer = Self.unwrapInboundIn(data)
16621691
let byteCount = buffer.readableBytes
16631692
self.currentSecondBytesSeen += byteCount
1664-
context.eventLoop.scheduleTask(in: .seconds(1)) {
1693+
context.eventLoop.assumeIsolated().scheduleTask(in: .seconds(1)) {
16651694
self.currentSecondBytesSeen -= byteCount
16661695
self.evaluatePause(context: loopBoundContext.value)
16671696
}

0 commit comments

Comments
 (0)
Please sign in to comment.