From e124d482b26cc0c63b3e5cde43da8f7771c88625 Mon Sep 17 00:00:00 2001 From: Johannes Weiss Date: Thu, 5 Sep 2019 09:14:24 +0100 Subject: [PATCH] make sure HTTPClient is shutdown Motivation: Right now, HTTPClient only asserts that it's shut down if it was started with its own EventLoopGroup. That however is weird because it's lifecycle model depends on the parameters you pass to `init`. Modifications: Always validate the lifecycle (in debug mode). Result: API makes more sense. --- Sources/AsyncHTTPClient/HTTPClient.swift | 7 +-- .../HTTPClientInternalTests.swift | 6 +-- .../HTTPClientTests.swift | 54 ++++++++++--------- 3 files changed, 32 insertions(+), 35 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 57807f1ec..040fc009c 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -66,12 +66,7 @@ public class HTTPClient { } deinit { - switch self.eventLoopGroupProvider { - case .shared: - return - case .createNew: - assert(self.isShutdown.load(), "Client not stopped before the deinit.") - } + assert(self.isShutdown.load(), "Client not shut down before the deinit. Please call client.syncShutdown() when no longer needed.") } /// Shuts down the client and `EventLoopGroup` if it was created by the client. diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift index 9ca4c26ed..0c650482f 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift @@ -77,7 +77,7 @@ class HTTPClientInternalTests: XCTestCase { let httpBin = HttpBin() let httpClient = HTTPClient(eventLoopGroupProvider: .createNew) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -107,7 +107,7 @@ class HTTPClientInternalTests: XCTestCase { let httpBin = HttpBin() let httpClient = HTTPClient(eventLoopGroupProvider: .createNew) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -168,7 +168,7 @@ class HTTPClientInternalTests: XCTestCase { let httpBin = HttpBin(channelPromise: promise) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index c3e8bf105..669ea9b2f 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -54,7 +54,7 @@ class HTTPClientTests: XCTestCase { let httpBin = HttpBin() let httpClient = HTTPClient(eventLoopGroupProvider: .createNew) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -67,7 +67,8 @@ class HTTPClientTests: XCTestCase { let elg = MultiThreadedEventLoopGroup(numberOfThreads: 8) let httpClient = HTTPClient(eventLoopGroupProvider: .shared(elg)) defer { - try! elg.syncShutdownGracefully() + XCTAssertNoThrow(try httpClient.syncShutdown()) + XCTAssertNoThrow(try elg.syncShutdownGracefully()) httpBin.shutdown() } @@ -85,7 +86,7 @@ class HTTPClientTests: XCTestCase { let httpBin = HttpBin() let httpClient = HTTPClient(eventLoopGroupProvider: .createNew) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -102,7 +103,7 @@ class HTTPClientTests: XCTestCase { let httpClient = HTTPClient(eventLoopGroupProvider: .createNew, configuration: HTTPClient.Configuration(certificateVerification: .none)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -115,7 +116,7 @@ class HTTPClientTests: XCTestCase { let httpClient = HTTPClient(eventLoopGroupProvider: .createNew, configuration: HTTPClient.Configuration(certificateVerification: .none)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -136,7 +137,7 @@ class HTTPClientTests: XCTestCase { configuration: HTTPClient.Configuration(certificateVerification: .none, followRedirects: true)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() httpsBin.shutdown() } @@ -154,7 +155,7 @@ class HTTPClientTests: XCTestCase { configuration: HTTPClient.Configuration(certificateVerification: .none, followRedirects: true)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -176,7 +177,7 @@ class HTTPClientTests: XCTestCase { let httpBin = HttpBin() let httpClient = HTTPClient(eventLoopGroupProvider: .createNew) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -187,7 +188,7 @@ class HTTPClientTests: XCTestCase { func testMultipleContentLengthHeaders() throws { let httpClient = HTTPClient(eventLoopGroupProvider: .createNew) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) } let httpBin = HttpBin() defer { @@ -208,7 +209,7 @@ class HTTPClientTests: XCTestCase { let httpBin = HttpBin() let httpClient = HTTPClient(eventLoopGroupProvider: .createNew) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -226,7 +227,7 @@ class HTTPClientTests: XCTestCase { let httpClient = HTTPClient(eventLoopGroupProvider: .createNew) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -242,7 +243,7 @@ class HTTPClientTests: XCTestCase { let httpClient = HTTPClient(eventLoopGroupProvider: .createNew, configuration: HTTPClient.Configuration(timeout: HTTPClient.Configuration.Timeout(read: .milliseconds(150)))) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -258,7 +259,7 @@ class HTTPClientTests: XCTestCase { let httpClient = HTTPClient(eventLoopGroupProvider: .createNew) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -274,7 +275,7 @@ class HTTPClientTests: XCTestCase { let httpClient = HTTPClient(eventLoopGroupProvider: .createNew) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -300,7 +301,7 @@ class HTTPClientTests: XCTestCase { configuration: .init(proxy: .server(host: "localhost", port: httpBin.port)) ) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } let res = try httpClient.get(url: "http://test/ok").wait() @@ -317,7 +318,7 @@ class HTTPClientTests: XCTestCase { ) ) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } let res = try httpClient.get(url: "https://test/ok").wait() @@ -328,7 +329,7 @@ class HTTPClientTests: XCTestCase { let httpBin = HttpBin() let httpClient = HTTPClient(eventLoopGroupProvider: .createNew) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -354,7 +355,7 @@ class HTTPClientTests: XCTestCase { configuration: HTTPClient.Configuration(certificateVerification: .none)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -371,7 +372,7 @@ class HTTPClientTests: XCTestCase { configuration: HTTPClient.Configuration(certificateVerification: .none, ignoreUncleanSSLShutdown: true)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -389,7 +390,7 @@ class HTTPClientTests: XCTestCase { configuration: HTTPClient.Configuration(certificateVerification: .none)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -407,7 +408,7 @@ class HTTPClientTests: XCTestCase { configuration: HTTPClient.Configuration(certificateVerification: .none)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -423,7 +424,7 @@ class HTTPClientTests: XCTestCase { configuration: HTTPClient.Configuration(certificateVerification: .none)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -440,7 +441,7 @@ class HTTPClientTests: XCTestCase { configuration: HTTPClient.Configuration(certificateVerification: .none, ignoreUncleanSSLShutdown: true)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -457,7 +458,7 @@ class HTTPClientTests: XCTestCase { configuration: HTTPClient.Configuration(certificateVerification: .none)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -474,7 +475,7 @@ class HTTPClientTests: XCTestCase { configuration: HTTPClient.Configuration(certificateVerification: .none, ignoreUncleanSSLShutdown: true)) defer { - try! httpClient.syncShutdown() + XCTAssertNoThrow(try httpClient.syncShutdown()) httpBin.shutdown() } @@ -491,7 +492,8 @@ class HTTPClientTests: XCTestCase { let httpClient = HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup), configuration: HTTPClient.Configuration(followRedirects: true)) defer { - try! eventLoopGroup.syncShutdownGracefully() + XCTAssertNoThrow(try httpClient.syncShutdown()) + XCTAssertNoThrow(try eventLoopGroup.syncShutdownGracefully()) httpBin.shutdown() }