From 4392120092cd4a8040ca9edc9b8e5d58d6d26da6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Helge=20He=C3=9F?= Date: Sun, 16 Oct 2022 13:13:45 +0200 Subject: [PATCH 1/4] Harden `SimplePool` for extensions Make sure to capture the notification subscription tokens in deinit, before dispatching out to main. Also used the proper extension notifications when running in one. --- .../ConnectionHandlers/SimplePool.swift | 57 ++++++++++++------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/Sources/Lighter/ConnectionHandlers/SimplePool.swift b/Sources/Lighter/ConnectionHandlers/SimplePool.swift index 1f7d2c3..529a056 100644 --- a/Sources/Lighter/ConnectionHandlers/SimplePool.swift +++ b/Sources/Lighter/ConnectionHandlers/SimplePool.swift @@ -3,16 +3,21 @@ // Copyright © 2022 ZeeZide GmbH. // +#if canImport(Foundation) // could drop this dep if required +import class Foundation.Bundle import class Foundation.NSLock import struct Foundation.Date import struct Foundation.TimeInterval import struct Foundation.URL import Dispatch import func SQLite3.sqlite3_close + #if os(iOS) import UIKit #endif +fileprivate let isAppExtension = Bundle.main.bundleURL.pathExtension == "appex" + extension SQLConnectionHandler { /** @@ -52,10 +57,13 @@ extension SQLConnectionHandler { { self.maxAge = maxAge self.maxPerConfiguration = maximumPoolSizePerConfiguration + super.init(url: url, readOnly: readOnly, writeTimeout: writeTimeout) #if os(iOS) - subscribeForAppStateNotifications() + DispatchQueue.main.async { + subscribeForAppStateNotifications() + } #endif } deinit { @@ -63,34 +71,41 @@ extension SQLConnectionHandler { closePooledConnections() #if os(iOS) - unsubscribeFromAppStateNotifications() + let fgSubscription = self.fgSubscription + let bgSubscription = self.bgSubscription + if fgSubscription != nil || bgSubscription != nil { + DispatchQueue.main.async { + let nc = NotificationCenter.default + if let token = fgSubscription { nc.removeObserver(token) } + if let token = bgSubscription { nc.removeObserver(token) } + } + } #endif } // MARK: - App State Handling - + #if os(iOS) private func subscribeForAppStateNotifications() { - DispatchQueue.main.async { - let nc = NotificationCenter.default - self.fgSubscription = nc.addObserver( - forName: UIApplication.willEnterForegroundNotification, object: nil, - queue: nil) { [weak self] _ in self?.willEnterForeground() } - self.bgSubscription = nc.addObserver( - forName: UIApplication.didEnterBackgroundNotification, object: nil, - queue: nil) { [weak self] _ in self?.didEnterBackground() } + let nc = NotificationCenter.default + if self.fgSubscription == nil { + let name = isAppExtension + ? NSExtensionHostWillEnterForegroundNotification + : UIApplication.willEnterForegroundNotification + self.fgSubscription = nc.addObserver(forName: name, object: nil, + queue: nil) { [weak self] _ in self?.willEnterForeground() } } - } - private func unsubscribeFromAppStateNotifications() { - DispatchQueue.main.async { - let nc = NotificationCenter.default - if let token = self.fgSubscription { nc.removeObserver(token) } - if let token = self.bgSubscription { nc.removeObserver(token) } - self.fgSubscription = nil - self.bgSubscription = nil + // TBD: Does background mean anything to us? + if self.bgSubscription == nil { + let name = isAppExtension + ? NSExtensionHostDidEnterBackgroundNotification + : UIApplication.didEnterBackgroundNotification + self.bgSubscription = nc.addObserver(forName: name, object: nil, + queue: nil) { [weak self] _ in self?.didEnterBackground() } } } + private func willEnterForeground() { lock.lock() allowPooling = true // re-enable pooling in case it was disabled @@ -102,7 +117,7 @@ extension SQLConnectionHandler { lock.unlock() closePooledConnections() } - #endif + #endif // os(iOS) // MARK: - Connection Handling @@ -206,3 +221,5 @@ extension SQLConnectionHandler { } } } + +#endif // canImport(Foundation) From e7ce553e75eb8b9dcaee46ec342b185c8d923a62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Helge=20He=C3=9F?= Date: Sun, 16 Oct 2022 13:17:57 +0200 Subject: [PATCH 2/4] Comments and beauty Clarify the bg behaviour. --- Sources/Lighter/ConnectionHandlers/SimplePool.swift | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Sources/Lighter/ConnectionHandlers/SimplePool.swift b/Sources/Lighter/ConnectionHandlers/SimplePool.swift index 529a056..f102d1d 100644 --- a/Sources/Lighter/ConnectionHandlers/SimplePool.swift +++ b/Sources/Lighter/ConnectionHandlers/SimplePool.swift @@ -89,19 +89,21 @@ extension SQLConnectionHandler { #if os(iOS) private func subscribeForAppStateNotifications() { let nc = NotificationCenter.default - if self.fgSubscription == nil { + if fgSubscription == nil { let name = isAppExtension ? NSExtensionHostWillEnterForegroundNotification : UIApplication.willEnterForegroundNotification - self.fgSubscription = nc.addObserver(forName: name, object: nil, + fgSubscription = nc.addObserver(forName: name, object: nil, queue: nil) { [weak self] _ in self?.willEnterForeground() } } - // TBD: Does background mean anything to us? - if self.bgSubscription == nil { + if bgSubscription == nil { + // When we go to the background, we can still operate, but won't pool + // connections anymore. This is because the watchdog can kill processes + // holding SQLite connections in a suspended state (to avoid deadlocks). let name = isAppExtension ? NSExtensionHostDidEnterBackgroundNotification : UIApplication.didEnterBackgroundNotification - self.bgSubscription = nc.addObserver(forName: name, object: nil, + bgSubscription = nc.addObserver(forName: name, object: nil, queue: nil) { [weak self] _ in self?.didEnterBackground() } } } From 951784d1762496911e54f5047422e51402dba594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Helge=20He=C3=9F?= Date: Mon, 31 Oct 2022 17:46:53 +0100 Subject: [PATCH 3/4] Attempt to fix crasher in pool dealloc I think I've seen issues w/ unregistering on different Qs from deinit's coming up. This is an attempt to fix this. --- .../ConnectionHandlers/SimplePool.swift | 130 ++++++++++-------- 1 file changed, 76 insertions(+), 54 deletions(-) diff --git a/Sources/Lighter/ConnectionHandlers/SimplePool.swift b/Sources/Lighter/ConnectionHandlers/SimplePool.swift index f102d1d..0852182 100644 --- a/Sources/Lighter/ConnectionHandlers/SimplePool.swift +++ b/Sources/Lighter/ConnectionHandlers/SimplePool.swift @@ -16,8 +16,6 @@ import func SQLite3.sqlite3_close import UIKit #endif -fileprivate let isAppExtension = Bundle.main.bundleURL.pathExtension == "appex" - extension SQLConnectionHandler { /** @@ -41,13 +39,8 @@ extension SQLConnectionHandler { private var caches = [ Configuration : [ Entry ] ]() private var gc : DispatchWorkItem? - #if os(iOS) // Q: .main - private var allowPooling = true - private var fgSubscription : NSObjectProtocol? - private var bgSubscription : NSObjectProtocol? - #else - private let allowPooling = true - #endif + private var allowPooling = true + private var lifecycle : AppLifecycleHandler? /// Initialize a simple pool. public init(url: URL, readOnly: Bool, @@ -61,64 +54,31 @@ extension SQLConnectionHandler { super.init(url: url, readOnly: readOnly, writeTimeout: writeTimeout) #if os(iOS) - DispatchQueue.main.async { - subscribeForAppStateNotifications() - } + lifecycle = AppLifecycleHandler(owner: self) + lifecycle?.resume() #endif } deinit { gc?.cancel(); gc = nil closePooledConnections() - - #if os(iOS) - let fgSubscription = self.fgSubscription - let bgSubscription = self.bgSubscription - if fgSubscription != nil || bgSubscription != nil { - DispatchQueue.main.async { - let nc = NotificationCenter.default - if let token = fgSubscription { nc.removeObserver(token) } - if let token = bgSubscription { nc.removeObserver(token) } - } - } - #endif + lifecycle?.suspend() } // MARK: - App State Handling #if os(iOS) - private func subscribeForAppStateNotifications() { - let nc = NotificationCenter.default - if fgSubscription == nil { - let name = isAppExtension - ? NSExtensionHostWillEnterForegroundNotification - : UIApplication.willEnterForegroundNotification - fgSubscription = nc.addObserver(forName: name, object: nil, - queue: nil) { [weak self] _ in self?.willEnterForeground() } + fileprivate func willEnterForeground() { + lock.lock() + allowPooling = true // re-enable pooling in case it was disabled + lock.unlock() } - if bgSubscription == nil { - // When we go to the background, we can still operate, but won't pool - // connections anymore. This is because the watchdog can kill processes - // holding SQLite connections in a suspended state (to avoid deadlocks). - let name = isAppExtension - ? NSExtensionHostDidEnterBackgroundNotification - : UIApplication.didEnterBackgroundNotification - bgSubscription = nc.addObserver(forName: name, object: nil, - queue: nil) { [weak self] _ in self?.didEnterBackground() } + fileprivate func didEnterBackground() { + lock.lock() + allowPooling = false // disable pooling + lock.unlock() + closePooledConnections() } - } - - private func willEnterForeground() { - lock.lock() - allowPooling = true // re-enable pooling in case it was disabled - lock.unlock() - } - private func didEnterBackground() { - lock.lock() - allowPooling = false // disable pooling - lock.unlock() - closePooledConnections() - } #endif // os(iOS) @@ -224,4 +184,66 @@ extension SQLConnectionHandler { } } +fileprivate final class AppLifecycleHandler: NSObject { + + private weak var owner : SQLConnectionHandler.SimplePool? + + init(owner: SQLConnectionHandler.SimplePool) { self.owner = owner } + + func resume() { + #if os(iOS) + let me = self // keep alive + DispatchQueue.main.async { + let nc = NotificationCenter.default + nc.addObserver(me, selector: #selector(Self.willEnterForeground(_:)), + name: Self.fgName, object: nil) + nc.addObserver(me, selector: #selector(Self.didEnterbackground(_:)), + name: Self.bgName, object: nil) + } + #endif + } + func suspend() { + #if os(iOS) + let me = self + DispatchQueue.main.async { + let nc = NotificationCenter.default + nc.removeObserver(me) + } + #endif + } + + #if os(iOS) + @objc private func willEnterForeground(_ notification: Notification) { + owner?.willEnterForeground() + } + @objc private func didEnterbackground(_ notification: Notification) { + owner?.didEnterBackground() + } + + private static let isAppExtension = + Bundle.main.bundleURL.pathExtension == "appex" + + private static let fgName : NSNotification.Name = { + if !isAppExtension { + return UIApplication.willEnterForegroundNotification + } + #if swift(>=5.7.1) + return NSNotification.Name.NSExtensionHostWillEnterForeground + #else + return NSExtensionHostWillEnterForegroundNotification + #endif + }() + private static let bgName : NSNotification.Name = { + if !isAppExtension { + return UIApplication.didEnterBackgroundNotification + } + #if swift(>=5.7.1) + return NSNotification.Name.NSExtensionHostDidEnterBackground + #else + return NSExtensionHostDidEnterBackgroundNotification + #endif + }() + #endif // os(iOS) +} + #endif // canImport(Foundation) From 5e13322d25c91b6d04ce09051aeff9ea185dda56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Helge=20He=C3=9F?= Date: Mon, 31 Oct 2022 17:50:12 +0100 Subject: [PATCH 4/4] Import NSObject to fix Linux build Does it have NSObject? It should :-) --- Sources/Lighter/ConnectionHandlers/SimplePool.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Lighter/ConnectionHandlers/SimplePool.swift b/Sources/Lighter/ConnectionHandlers/SimplePool.swift index 0852182..d975b6f 100644 --- a/Sources/Lighter/ConnectionHandlers/SimplePool.swift +++ b/Sources/Lighter/ConnectionHandlers/SimplePool.swift @@ -4,6 +4,7 @@ // #if canImport(Foundation) // could drop this dep if required +import class Foundation.NSObject import class Foundation.Bundle import class Foundation.NSLock import struct Foundation.Date