Fixed an issue where a manager would not be properly shut down unless…

… it was cancelled
mochidev · Dec 18, 2024 · 895537e · 895537e
1 parent a23acd0
commit 895537e
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 42 deletions.
diff --git 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
@@ -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,25 +88,13 @@ 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()
-            }
         }
     }
 
     @Suite("VAPID Key Retrieval") struct VAPIDKeyRetrieval {
         @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()
-            }
         }
     }