Skip to content

Commit 9ee5c82

Browse files
committed
Add test to verify that we actually share the same thread pool across all delegates for a given HTTPClient
1 parent c3c68b2 commit 9ee5c82

File tree

4 files changed

+62
-9
lines changed

4 files changed

+62
-9
lines changed

Sources/AsyncHTTPClient/FileDownloadDelegate.swift

+9-9
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate {
3030
public typealias Response = Progress
3131

3232
private let filePath: String
33-
private var io: NonBlockingFileIO?
33+
private(set) var fileIOThreadPool: NIOThreadPool?
3434
private let reportHead: ((HTTPResponseHead) -> Void)?
3535
private let reportProgress: ((Progress) -> Void)?
3636

@@ -80,11 +80,11 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate {
8080
reportProgress: ((Progress) -> Void)? = nil
8181
) throws {
8282
if let pool = pool {
83-
self.io = NonBlockingFileIO(threadPool: pool)
83+
self.fileIOThreadPool = pool
8484
} else {
8585
// we should use the shared thread pool from the HTTPClient which
8686
// we will get from the `HTTPClient.Task`
87-
self.io = nil
87+
self.fileIOThreadPool = nil
8888
}
8989

9090
self.filePath = path
@@ -111,15 +111,15 @@ public final class FileDownloadDelegate: HTTPClientResponseDelegate {
111111
task: HTTPClient.Task<Response>,
112112
_ buffer: ByteBuffer
113113
) -> EventLoopFuture<Void> {
114-
let io: NonBlockingFileIO = {
115-
guard let io = self.io else {
114+
let threadPool: NIOThreadPool = {
115+
guard let pool = self.fileIOThreadPool else {
116116
let pool = task.fileIOThreadPool
117-
let io = NonBlockingFileIO(threadPool: pool)
118-
self.io = io
119-
return io
117+
self.fileIOThreadPool = pool
118+
return pool
120119
}
121-
return io
120+
return pool
122121
}()
122+
let io = NonBlockingFileIO(threadPool: threadPool)
123123
self.progress.receivedBytes += buffer.readableBytes
124124
self.reportProgress?(self.progress)
125125

Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ extension HTTPClientInternalTests {
3636
("testConnectErrorCalloutOnCorrectEL", testConnectErrorCalloutOnCorrectEL),
3737
("testInternalRequestURI", testInternalRequestURI),
3838
("testHasSuffix", testHasSuffix),
39+
("testSharedThreadPoolIsIdenticalForAllDelegates", testSharedThreadPoolIsIdenticalForAllDelegates),
3940
]
4041
}
4142
}

Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift

+35
Original file line numberDiff line numberDiff line change
@@ -554,4 +554,39 @@ class HTTPClientInternalTests: XCTestCase {
554554
XCTAssertFalse(elements.hasSuffix([0, 0, 0]))
555555
}
556556
}
557+
558+
/// test to verify that we actually share the same thread pool across all ``FileDownloadDelegate``s for a given ``HTTPClient``
559+
func testSharedThreadPoolIsIdenticalForAllDelegates() throws {
560+
let httpBin = HTTPBin()
561+
let httpClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup))
562+
defer {
563+
XCTAssertNoThrow(try httpClient.syncShutdown(requiresCleanClose: true))
564+
XCTAssertNoThrow(try httpBin.shutdown())
565+
}
566+
var request = try Request(url: "http://localhost:\(httpBin.port)/events/10/content-length")
567+
request.headers.add(name: "Accept", value: "text/event-stream")
568+
569+
let filePaths = (0..<10).map { _ in
570+
TemporaryFileHelpers.makeTemporaryFilePath()
571+
}
572+
defer {
573+
for filePath in filePaths {
574+
TemporaryFileHelpers.removeTemporaryFile(at: filePath)
575+
}
576+
}
577+
let delegates = try filePaths.map {
578+
try FileDownloadDelegate(path: $0)
579+
}
580+
581+
let resultFutures = delegates.map { delegate in
582+
httpClient.execute(
583+
request: request,
584+
delegate: delegate
585+
).futureResult
586+
}
587+
_ = try EventLoopFuture.whenAllSucceed(resultFutures, on: self.clientGroup.next()).wait()
588+
let threadPools = delegates.map { $0.fileIOThreadPool }
589+
let firstThreadPool = threadPools.first ?? nil
590+
XCTAssert(threadPools.dropFirst().allSatisfy { $0 === firstThreadPool })
591+
}
557592
}

Tests/AsyncHTTPClientTests/HTTPClientTestUtils.swift

+17
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,23 @@ enum TemporaryFileHelpers {
283283
return try body(path)
284284
}
285285

286+
internal static func makeTemporaryFilePath(
287+
directory: String = temporaryDirectory
288+
) -> String {
289+
let (fd, path) = self.openTemporaryFile()
290+
close(fd)
291+
try! FileManager.default.removeItem(atPath: path)
292+
return path
293+
}
294+
295+
internal static func removeTemporaryFile(
296+
at path: String
297+
) {
298+
if FileManager.default.fileExists(atPath: path) {
299+
try? FileManager.default.removeItem(atPath: path)
300+
}
301+
}
302+
286303
internal static func fileSize(path: String) throws -> Int? {
287304
return try FileManager.default.attributesOfItem(atPath: path)[.size] as? Int
288305
}

0 commit comments

Comments
 (0)