From 1a53f0fe72189274f0a15f9f725980643d0be8a5 Mon Sep 17 00:00:00 2001 From: Alexander Smarus Date: Tue, 19 Dec 2023 17:51:12 +0200 Subject: [PATCH] Add tests for `URLSession._MultiHandle` Windows issue This adds test cases for #4791. Test HTTPServer needs to be reconfigured for one of the test scenarios, so there are options now. --- Tests/Foundation/HTTPServer.swift | 56 ++++++++--- Tests/Foundation/Tests/TestURLSession.swift | 105 ++++++++++++++++++-- 2 files changed, 143 insertions(+), 18 deletions(-) diff --git a/Tests/Foundation/HTTPServer.swift b/Tests/Foundation/HTTPServer.swift index 5af9fb9c526..a3ec19cd830 100644 --- a/Tests/Foundation/HTTPServer.swift +++ b/Tests/Foundation/HTTPServer.swift @@ -99,7 +99,7 @@ class _TCPSocket: CustomStringConvertible { listening = false } - init(port: UInt16?) throws { + init(port: UInt16?, backlog: Int32) throws { listening = true self.port = 0 @@ -124,7 +124,7 @@ class _TCPSocket: CustomStringConvertible { try socketAddress.withMemoryRebound(to: sockaddr.self, capacity: MemoryLayout.size, { let addr = UnsafePointer($0) _ = try attempt("bind", valid: isZero, bind(_socket, addr, socklen_t(MemoryLayout.size))) - _ = try attempt("listen", valid: isZero, listen(_socket, SOMAXCONN)) + _ = try attempt("listen", valid: isZero, listen(_socket, backlog)) }) var actualSA = sockaddr_in() @@ -295,8 +295,8 @@ class _HTTPServer: CustomStringConvertible { let tcpSocket: _TCPSocket var port: UInt16 { tcpSocket.port } - init(port: UInt16?) throws { - tcpSocket = try _TCPSocket(port: port) + init(port: UInt16?, backlog: Int32 = SOMAXCONN) throws { + tcpSocket = try _TCPSocket(port: port, backlog: backlog) } init(socket: _TCPSocket) { @@ -1094,15 +1094,32 @@ enum InternalServerError : Error { case badHeaders } +extension LoopbackServerTest { + struct Options { + var serverBacklog: Int32 + var isAsynchronous: Bool + + static let `default` = Options(serverBacklog: SOMAXCONN, isAsynchronous: true) + } +} -class LoopbackServerTest : XCTestCase { +class LoopbackServerTest : XCTestCase { private static let staticSyncQ = DispatchQueue(label: "org.swift.TestFoundation.HTTPServer.StaticSyncQ") private static var _serverPort: Int = -1 private static var _serverActive = false private static var testServer: _HTTPServer? = nil - - + private static var _options: Options = .default + + static var options: Options { + get { + return staticSyncQ.sync { _options } + } + set { + staticSyncQ.sync { _options = newValue } + } + } + static var serverPort: Int { get { return staticSyncQ.sync { _serverPort } @@ -1119,12 +1136,20 @@ class LoopbackServerTest : XCTestCase { override class func setUp() { super.setUp() + Self.startServer() + } + override class func tearDown() { + Self.stopServer() + super.tearDown() + } + + static func startServer() { var _serverPort = 0 let dispatchGroup = DispatchGroup() func runServer() throws { - testServer = try _HTTPServer(port: nil) + testServer = try _HTTPServer(port: nil, backlog: options.serverBacklog) _serverPort = Int(testServer!.port) serverActive = true dispatchGroup.leave() @@ -1132,7 +1157,8 @@ class LoopbackServerTest : XCTestCase { while serverActive { do { let httpServer = try testServer!.listen() - globalDispatchQueue.async { + + func handleRequest() { let subServer = TestURLSessionServer(httpServer: httpServer) do { try subServer.readAndRespond() @@ -1140,6 +1166,12 @@ class LoopbackServerTest : XCTestCase { NSLog("readAndRespond: \(error)") } } + + if options.isAsynchronous { + globalDispatchQueue.async(execute: handleRequest) + } else { + handleRequest() + } } catch { if (serverActive) { // Ignore errors thrown on shutdown NSLog("httpServer: \(error)") @@ -1165,11 +1197,11 @@ class LoopbackServerTest : XCTestCase { fatalError("Timedout waiting for server to be ready") } serverPort = _serverPort + debugLog("Listening on \(serverPort)") } - - override class func tearDown() { + + static func stopServer() { serverActive = false try? testServer?.stop() - super.tearDown() } } diff --git a/Tests/Foundation/Tests/TestURLSession.swift b/Tests/Foundation/Tests/TestURLSession.swift index 8c048555892..f53775cdc25 100644 --- a/Tests/Foundation/Tests/TestURLSession.swift +++ b/Tests/Foundation/Tests/TestURLSession.swift @@ -495,21 +495,101 @@ class TestURLSession: LoopbackServerTest { waitForExpectations(timeout: 30) } - func test_timeoutInterval() { + func test_httpTimeout() { let config = URLSessionConfiguration.default config.timeoutIntervalForRequest = 10 - let urlString = "http://127.0.0.1:-1/Peru" + let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/Peru" let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) let expect = expectation(description: "GET \(urlString): will timeout") - var req = URLRequest(url: URL(string: "http://127.0.0.1:-1/Peru")!) + var req = URLRequest(url: URL(string: urlString)!) + req.setValue("3", forHTTPHeaderField: "x-pause") req.timeoutInterval = 1 let task = session.dataTask(with: req) { (data, _, error) -> Void in defer { expect.fulfill() } - XCTAssertNotNil(error) + XCTAssertEqual((error as? URLError)?.code, .timedOut, "Task should fail with URLError.timedOut error") } task.resume() + waitForExpectations(timeout: 30) + } + + func test_connectTimeout() { + // Reconfigure http server for this specific scenario: + // a slow request keeps web server busy, while other + // request times out on connection attempt. + Self.stopServer() + Self.options = Options(serverBacklog: 1, isAsynchronous: false) + Self.startServer() + + let config = URLSessionConfiguration.default + let slowUrlString = "http://127.0.0.1:\(TestURLSession.serverPort)/Peru" + let fastUrlString = "http://127.0.0.1:\(TestURLSession.serverPort)/Italy" + let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) + let slowReqExpect = expectation(description: "GET \(slowUrlString): will complete") + let fastReqExpect = expectation(description: "GET \(fastUrlString): will timeout") + + var slowReq = URLRequest(url: URL(string: slowUrlString)!) + slowReq.setValue("3", forHTTPHeaderField: "x-pause") + + var fastReq = URLRequest(url: URL(string: fastUrlString)!) + fastReq.timeoutInterval = 1 + + let slowTask = session.dataTask(with: slowReq) { (data, _, error) -> Void in + slowReqExpect.fulfill() + } + let fastTask = session.dataTask(with: fastReq) { (data, _, error) -> Void in + defer { fastReqExpect.fulfill() } + XCTAssertEqual((error as? URLError)?.code, .timedOut, "Task should fail with URLError.timedOut error") + } + slowTask.resume() + Thread.sleep(forTimeInterval: 0.1) // Give slow task some time to start + fastTask.resume() waitForExpectations(timeout: 30) + + // Reconfigure http server back to default settings + Self.stopServer() + Self.options = .default + Self.startServer() + } + + func test_repeatedRequestsStress() throws { + let config = URLSessionConfiguration.default + let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/Peru" + let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) + let req = URLRequest(url: URL(string: urlString)!) + + var requestsLeft = 3000 + let expect = expectation(description: "\(requestsLeft) x GET \(urlString)") + + func doRequest(completion: @escaping () -> Void) { + // We only care about completion of one of the tasks, + // so we could move to next cycle. + // Some overlapping would happen and that's what we + // want actually to provoke issue with socket reuse + // on Windows. + let task = session.dataTask(with: req) { (_, _, _) -> Void in + } + task.resume() + let task2 = session.dataTask(with: req) { (_, _, _) -> Void in + } + task2.resume() + let task3 = session.dataTask(with: req) { (_, _, _) -> Void in + completion() + } + task3.resume() + } + + func requestCompletion() { + requestsLeft -= 1 + guard requestsLeft > 0 else { + expect.fulfill() + return + } + doRequest(completion: requestCompletion) + } + doRequest(completion: requestCompletion) + + waitForExpectations(timeout: 30) } func test_httpRedirectionWithCode300() throws { @@ -2026,7 +2106,7 @@ class TestURLSession: LoopbackServerTest { #endif static var allTests: [(String, (TestURLSession) -> () throws -> Void)] { - var retVal = [ + var retVal:[(String, (TestURLSession) -> () throws -> Void)] = [ ("test_dataTaskWithURL", test_dataTaskWithURL), ("test_dataTaskWithURLRequest", test_dataTaskWithURLRequest), ("test_dataTaskWithURLCompletionHandler", test_dataTaskWithURLCompletionHandler), @@ -2049,7 +2129,6 @@ class TestURLSession: LoopbackServerTest { ("test_taskTimeout", test_taskTimeout), ("test_verifyRequestHeaders", test_verifyRequestHeaders), ("test_verifyHttpAdditionalHeaders", test_verifyHttpAdditionalHeaders), - ("test_timeoutInterval", test_timeoutInterval), ("test_httpRedirectionWithCode300", test_httpRedirectionWithCode300), ("test_httpRedirectionWithCode301_302", test_httpRedirectionWithCode301_302), ("test_httpRedirectionWithCode303", test_httpRedirectionWithCode303), @@ -2098,6 +2177,7 @@ class TestURLSession: LoopbackServerTest { /* ⚠️ */ testExpectedToFail(test_noDoubleCallbackWhenCancellingAndProtocolFailsFast, "This test crashes nondeterministically: https://bugs.swift.org/browse/SR-11310")), /* ⚠️ */ ("test_cancelledTasksCannotBeResumed", testExpectedToFail(test_cancelledTasksCannotBeResumed, "Breaks on Ubuntu 18.04")), ] + #if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT if #available(macOS 12.0, *) { retVal.append(contentsOf: [ ("test_webSocket", asyncTest(test_webSocket)), @@ -2106,6 +2186,19 @@ class TestURLSession: LoopbackServerTest { ("test_webSocketSemiAbruptClose", asyncTest(test_webSocketSemiAbruptClose)), ]) } + #endif + #if os(Windows) + retVal.append(contentsOf: [ + ("test_httpTimeout", testExpectedToFail(test_httpTimeout, "Crashes: https://github.com/apple/swift-corelibs-foundation/issues/4791")), + ("test_connectTimeout", testExpectedToFail(test_connectTimeout, "Crashes: https://github.com/apple/swift-corelibs-foundation/issues/4791")), + ("test_repeatedRequestsStress", testExpectedToFail(test_repeatedRequestsStress, "Crashes: https://github.com/apple/swift-corelibs-foundation/issues/4791")), + ]) + #else + retVal.append(contentsOf: [ + ("test_httpTimeout", test_httpTimeout), + ("test_connectTimeout", test_connectTimeout), + ]) + #endif return retVal }