From 896ee259f21ba8200bd418d6097c1598a3a3e359 Mon Sep 17 00:00:00 2001 From: Dimitri Bouniol Date: Wed, 18 Dec 2024 03:37:26 -0800 Subject: [PATCH 1/2] Fixed an issue where a manager would not be properly shut down unless it was cancelled --- Sources/WebPush/WebPushManager.swift | 15 +++++++ Tests/WebPushTests/WebPushManagerTests.swift | 42 -------------------- 2 files changed, 15 insertions(+), 42 deletions(-) diff --git a/Sources/WebPush/WebPushManager.swift b/Sources/WebPush/WebPushManager.swift index bdaa825..78066f5 100644 --- a/Sources/WebPush/WebPushManager.swift +++ b/Sources/WebPush/WebPushManager.swift @@ -42,6 +42,9 @@ public actor WebPushManager: Sendable { /// The internal executor to use when delivering messages. var executor: Executor + /// An internal flag indicating if a manager was shutdown already. + var didShutdown = false + /// An internal lookup of keys as provided by the VAPID configuration. let vapidKeyLookup: [VAPID.Key.ID : VAPID.Key] @@ -121,6 +124,12 @@ public actor WebPushManager: Sendable { self.executor = executor } + deinit { + if !didShutdown, case let .httpClient(httpClient) = executor { + try? httpClient.syncShutdown() + } + } + /// Load an up-to-date Authorization header for the specified endpoint and signing key combo. /// - Parameters: /// - endpoint: The endpoint we'll be contacting to send push messages for a given subscriber. @@ -448,6 +457,11 @@ public actor WebPushManager: Sendable { extension WebPushManager: Service { public func run() async throws { logger.info("Starting up WebPushManager") + guard !didShutdown else { + assertionFailure("The manager was already shutdown and cannot be started.") + logger.error("The manager was already shutdown and cannot be started. Run your application server in debug mode to catch this.") + return + } try await withTaskCancellationOrGracefulShutdownHandler { try await gracefulShutdown() } onCancelOrGracefulShutdown: { [logger, executor] in @@ -462,6 +476,7 @@ extension WebPushManager: Service { ]) } } + didShutdown = true } } diff --git a/Tests/WebPushTests/WebPushManagerTests.swift b/Tests/WebPushTests/WebPushManagerTests.swift index 510c1ce..7c3b0c2 100644 --- a/Tests/WebPushTests/WebPushManagerTests.swift +++ b/Tests/WebPushTests/WebPushManagerTests.swift @@ -80,12 +80,6 @@ struct WebPushManagerTests { .mockedKeyID2 : .mockedKey2, .mockedKeyID3 : .mockedKey3, ]) - await withThrowingTaskGroup(of: Void.self) { group in - group.addTask { - try await manager.run() - } - group.cancelAll() - } } /// This is needed to cover the `uniquingKeysWith` safety call completely. @@ -94,12 +88,6 @@ struct WebPushManagerTests { configuration.unsafeUpdateKeys(primaryKey: .mockedKey1, keys: [.mockedKey1], deprecatedKeys: [.mockedKey1]) let manager = WebPushManager(vapidConfiguration: configuration) #expect(await manager.vapidKeyLookup == [.mockedKeyID1 : .mockedKey1]) - await withThrowingTaskGroup(of: Void.self) { group in - group.addTask { - try await manager.run() - } - group.cancelAll() - } } } @@ -107,12 +95,6 @@ struct WebPushManagerTests { @Test func retrievesPrimaryKey() async { let manager = WebPushManager(vapidConfiguration: .mockedConfiguration) #expect(manager.nextVAPIDKeyID == .mockedKeyID1) - await withThrowingTaskGroup(of: Void.self) { group in - group.addTask { - try await manager.run() - } - group.cancelAll() - } } @Test func alwaysRetrievesPrimaryKey() async throws { @@ -122,12 +104,6 @@ struct WebPushManagerTests { for _ in 0..<100_000 { #expect(manager.nextVAPIDKeyID == .mockedKeyID1) } - await withThrowingTaskGroup(of: Void.self) { group in - group.addTask { - try await manager.run() - } - group.cancelAll() - } } @Test func retrievesFallbackKeys() async throws { @@ -139,12 +115,6 @@ struct WebPushManagerTests { keyCounts[manager.nextVAPIDKeyID, default: 0] += 1 } #expect(abs(keyCounts[.mockedKeyID1, default: 0] - keyCounts[.mockedKeyID2, default: 0]) < 1_000) /// If this test fails, increase this number accordingly - await withThrowingTaskGroup(of: Void.self) { group in - group.addTask { - try await manager.run() - } - group.cancelAll() - } } @Test func neverRetrievesDeprecatedKeys() async throws { @@ -154,12 +124,6 @@ struct WebPushManagerTests { for _ in 0..<100_000 { #expect(manager.nextVAPIDKeyID != .mockedKeyID3) } - await withThrowingTaskGroup(of: Void.self) { group in - group.addTask { - try await manager.run() - } - group.cancelAll() - } } @Test func keyStatus() async throws { @@ -170,12 +134,6 @@ struct WebPushManagerTests { #expect(manager.keyStatus(for: .mockedKeyID2) == .valid) #expect(manager.keyStatus(for: .mockedKeyID3) == .deprecated) #expect(manager.keyStatus(for: .mockedKeyID4) == .unknown) - await withThrowingTaskGroup(of: Void.self) { group in - group.addTask { - try await manager.run() - } - group.cancelAll() - } } } From 7fa9c7c00d5cda429fb72830a3d5972071ecc7d4 Mon Sep 17 00:00:00 2001 From: Dimitri Bouniol Date: Wed, 18 Dec 2024 03:38:30 -0800 Subject: [PATCH 2/2] Updated suggested version in readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 44397b1..0e740ef 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ Please check the [releases](https://github.com/mochidev/swift-webpush/releases) dependencies: [ .package( url: "https://github.com/mochidev/swift-webpush.git", - .upToNextMinor(from: "0.2.1") + .upToNextMinor(from: "0.3.0") ), ], ...