fix: resolve critical concurrency deadlocks and main-thread blocking (#399)

datlechin · web-flow · commit f82d9408b113 · 2026-03-21T18:25:07.000+07:00
* fix: resolve critical concurrency deadlocks and main-thread blocking

- SSH-1 (critical): replace DispatchSemaphore.wait() with withCheckedContinuation in HostKeyVerifier
- SVC-3 (high): remove DispatchQueue.main.sync from SQLFormatterService, pass dialect as parameter
- DB-3/PLG-1 (high): remove synchronous loadPendingPlugins() fallback, throw error instead
- PLG-2 (medium): move SecStaticCodeCheckValidity to background path
- SVC-1 (high): move SyncCoordinator storage I/O to Task.detached
- APP-3.5 (high): protect waitForConnection continuation with OSAllocatedUnfairLock
- APP-4.1 (high): consolidate AppState sync into single syncAppStateWithCurrentSession()

* fix: guarantee isSuppressed reset with defer, document EmptyDialect visibility

* fix: address CodeRabbit review comments

- Use consistent user-facing identifier in pluginNotLoaded error
- Add explicit internal access to EmptyDialect
- Replace DispatchQueue.main.async with MainActor.run in HostKeyVerifier
- Log settings decode errors instead of silently swallowing with try?
diff --git a/TablePro/AppDelegate+ConnectionHandler.swift b/TablePro/AppDelegate+ConnectionHandler.swift
@@ -329,13 +329,17 @@ extension AppDelegate {
     }
 
     private func waitForConnection(timeout: Duration) async {
+        let didResume = OSAllocatedUnfairLock(initialState: false)
         await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in
-            var didResume = false
             var observer: NSObjectProtocol?
 
             func resumeOnce() {
-                guard !didResume else { return }
-                didResume = true
+                let shouldResume = didResume.withLock { alreadyResumed -> Bool in
+                    if alreadyResumed { return false }
+                    alreadyResumed = true
+                    return true
+                }
+                guard shouldResume else { return }
                 if let obs = observer {
                     NotificationCenter.default.removeObserver(obs)
                 }
diff --git a/TablePro/ContentView.swift b/TablePro/ContentView.swift
@@ -92,12 +92,6 @@ struct ContentView: View {
                                 payload: payload
                             )
                         }
-                        AppState.shared.isConnected = true
-                        AppState.shared.safeModeLevel = session.connection.safeModeLevel
-                        AppState.shared.editorLanguage = PluginManager.shared.editorLanguage(for: session.connection.type)
-                        AppState.shared.currentDatabaseType = session.connection.type
-                        AppState.shared.supportsDatabaseSwitching = PluginManager.shared.supportsDatabaseSwitching(
-                            for: session.connection.type)
                     }
                 } else {
                     currentSession = nil
@@ -130,20 +124,7 @@ struct ContentView: View {
                     }()
                 guard isOurWindow else { return }
 
-                if let session = DatabaseManager.shared.activeSessions[connectionId] {
-                    AppState.shared.isConnected = true
-                    AppState.shared.safeModeLevel = session.connection.safeModeLevel
-                    AppState.shared.editorLanguage = PluginManager.shared.editorLanguage(for: session.connection.type)
-                    AppState.shared.currentDatabaseType = session.connection.type
-                    AppState.shared.supportsDatabaseSwitching = PluginManager.shared.supportsDatabaseSwitching(
-                        for: session.connection.type)
-                } else {
-                    AppState.shared.isConnected = false
-                    AppState.shared.safeModeLevel = .silent
-                    AppState.shared.editorLanguage = .sql
-                    AppState.shared.currentDatabaseType = nil
-                    AppState.shared.supportsDatabaseSwitching = true
-                }
+                syncAppStateWithCurrentSession()
             }
             .onChange(of: sessionState?.toolbarState.safeModeLevel) { _, newLevel in
                 if let level = newLevel {
@@ -349,11 +330,7 @@ struct ContentView: View {
                 sessionState = nil
                 currentSession = nil
                 columnVisibility = .detailOnly
-                AppState.shared.isConnected = false
-                AppState.shared.safeModeLevel = .silent
-                AppState.shared.editorLanguage = .sql
-                AppState.shared.currentDatabaseType = nil
-                AppState.shared.supportsDatabaseSwitching = true
+                syncAppStateWithCurrentSession()
 
                 let tabbingId = "com.TablePro.main.\(sid.uuidString)"
                 DispatchQueue.main.async {
@@ -379,12 +356,27 @@ struct ContentView: View {
                 payload: payload
             )
         }
-        AppState.shared.isConnected = true
-        AppState.shared.safeModeLevel = newSession.connection.safeModeLevel
-        AppState.shared.editorLanguage = PluginManager.shared.editorLanguage(for: newSession.connection.type)
-        AppState.shared.currentDatabaseType = newSession.connection.type
-        AppState.shared.supportsDatabaseSwitching = PluginManager.shared.supportsDatabaseSwitching(
-            for: newSession.connection.type)
+    }
+
+    /// Single authoritative source for syncing AppState fields with the current session.
+    /// Called from `windowDidBecomeKey` (the correct trigger for per-window AppState)
+    /// and from `handleConnectionStatusChange` on disconnect cleanup.
+    private func syncAppStateWithCurrentSession() {
+        let connectionId = payload?.connectionId ?? currentSession?.id
+        if let connectionId, let session = DatabaseManager.shared.activeSessions[connectionId] {
+            AppState.shared.isConnected = true
+            AppState.shared.safeModeLevel = session.connection.safeModeLevel
+            AppState.shared.editorLanguage = PluginManager.shared.editorLanguage(for: session.connection.type)
+            AppState.shared.currentDatabaseType = session.connection.type
+            AppState.shared.supportsDatabaseSwitching = PluginManager.shared.supportsDatabaseSwitching(
+                for: session.connection.type)
+        } else {
+            AppState.shared.isConnected = false
+            AppState.shared.safeModeLevel = .silent
+            AppState.shared.editorLanguage = .sql
+            AppState.shared.currentDatabaseType = nil
+            AppState.shared.supportsDatabaseSwitching = true
+        }
     }
 
     // MARK: - Actions
diff --git a/TablePro/Core/Database/DatabaseDriver.swift b/TablePro/Core/Database/DatabaseDriver.swift
@@ -317,13 +317,12 @@ extension DatabaseDriver {
 enum DatabaseDriverFactory {
     static func createDriver(for connection: DatabaseConnection) throws -> DatabaseDriver {
         let pluginId = connection.type.pluginTypeId
-        // If the plugin isn't registered yet and background loading hasn't finished,
-        // fall back to synchronous loading for this critical code path.
-        if PluginManager.shared.driverPlugins[pluginId] == nil,
-           !PluginManager.shared.hasFinishedInitialLoad {
-            PluginManager.shared.loadPendingPlugins()
-        }
         guard let plugin = PluginManager.shared.driverPlugins[pluginId] else {
+            // If background loading hasn't finished yet, throw a specific error
+            // instead of blocking the main thread with synchronous plugin loading.
+            if !PluginManager.shared.hasFinishedInitialLoad {
+                throw PluginError.pluginNotLoaded(connection.type.rawValue)
+            }
             if connection.type.isDownloadablePlugin {
                 throw PluginError.pluginNotInstalled(connection.type.rawValue)
             }
diff --git a/TablePro/Core/Plugins/PluginError.swift b/TablePro/Core/Plugins/PluginError.swift
@@ -20,6 +20,7 @@ enum PluginError: LocalizedError {
     case pluginNotInstalled(String)
     case incompatibleWithCurrentApp(minimumRequired: String)
     case invalidDescriptor(pluginId: String, reason: String)
+    case pluginNotLoaded(String)
 
     var errorDescription: String? {
         switch self {
@@ -51,6 +52,8 @@ enum PluginError: LocalizedError {
             return String(localized: "This plugin requires TablePro \(minimumRequired) or later")
         case .invalidDescriptor(let pluginId, let reason):
             return String(localized: "Plugin '\(pluginId)' has an invalid descriptor: \(reason)")
+        case .pluginNotLoaded(let databaseType):
+            return String(localized: "The \(databaseType) driver plugin is still loading. Please try again in a moment.")
         }
     }
 }
diff --git a/TablePro/Core/Plugins/PluginManager.swift b/TablePro/Core/Plugins/PluginManager.swift
@@ -142,6 +142,14 @@ final class PluginManager {
                     logger.error("User plugin \(entry.url.lastPathComponent) has outdated PluginKit v\(pluginKitVersion)")
                     continue
                 }
+
+                // Verify code signature off the main thread (disk I/O in SecStaticCodeCheckValidity)
+                do {
+                    try verifyCodeSignatureStatic(bundle: bundle)
+                } catch {
+                    logger.error("Code signature verification failed for \(entry.url.lastPathComponent): \(error.localizedDescription)")
+                    continue
+                }
             }
 
             // Heavy I/O: dynamic linker resolution, C bridge library initialization
@@ -308,7 +316,8 @@ final class PluginManager {
                     current: pluginKitVersion
                 )
             }
-            try verifyCodeSignature(bundle: bundle)
+            // Code signature verification is deferred to loadBundlesOffMain()
+            // to avoid blocking the main thread with SecStaticCodeCheckValidity disk I/O.
         }
 
         pendingPluginURLs.append((url: url, source: source))
@@ -1048,13 +1057,21 @@ final class PluginManager {
     private static let signingTeamId = "D7HJ5TFYCU"
 
     private func createSigningRequirement() -> SecRequirement? {
+        Self.createSigningRequirementStatic()
+    }
+
+    nonisolated private static func createSigningRequirementStatic() -> SecRequirement? {
         var requirement: SecRequirement?
-        let requirementString = "anchor apple generic and certificate leaf[subject.OU] = \"\(Self.signingTeamId)\"" as CFString
+        let requirementString = "anchor apple generic and certificate leaf[subject.OU] = \"\(signingTeamId)\"" as CFString
         SecRequirementCreateWithString(requirementString, SecCSFlags(), &requirement)
         return requirement
     }
 
     private func verifyCodeSignature(bundle: Bundle) throws {
+        try Self.verifyCodeSignatureStatic(bundle: bundle)
+    }
+
+    nonisolated private static func verifyCodeSignatureStatic(bundle: Bundle) throws {
         var staticCode: SecStaticCode?
         let createStatus = SecStaticCodeCreateWithPath(
             bundle.bundleURL as CFURL,
@@ -1064,11 +1081,11 @@ final class PluginManager {
 
         guard createStatus == errSecSuccess, let code = staticCode else {
             throw PluginError.signatureInvalid(
-                detail: Self.describeOSStatus(createStatus)
+                detail: describeOSStatus(createStatus)
             )
         }
 
-        let requirement = createSigningRequirement()
+        let requirement = createSigningRequirementStatic()
 
         let checkStatus = SecStaticCodeCheckValidity(
             code,
@@ -1078,12 +1095,12 @@ final class PluginManager {
 
         guard checkStatus == errSecSuccess else {
             throw PluginError.signatureInvalid(
-                detail: Self.describeOSStatus(checkStatus)
+                detail: describeOSStatus(checkStatus)
             )
         }
     }
 
-    private static func describeOSStatus(_ status: OSStatus) -> String {
+    nonisolated private static func describeOSStatus(_ status: OSStatus) -> String {
         switch status {
         case -67_062: return "bundle is not signed"
         case -67_061: return "code signature is invalid"
diff --git a/TablePro/Core/SSH/HostKeyVerifier.swift b/TablePro/Core/SSH/HostKeyVerifier.swift
@@ -15,8 +15,7 @@ internal enum HostKeyVerifier {
     private static let logger = Logger(subsystem: "com.TablePro", category: "HostKeyVerifier")
 
     /// Verify the host key, prompting the user if needed.
-    /// This method blocks the calling thread while showing UI prompts.
-    /// Must be called from a background thread.
+    /// Uses `withCheckedContinuation` to await UI prompts without blocking the cooperative thread pool.
     /// - Parameters:
     ///   - keyData: The raw host key bytes from the SSH session
     ///   - keyType: The key type string (e.g. "ssh-rsa", "ssh-ed25519")
@@ -28,7 +27,7 @@ internal enum HostKeyVerifier {
         keyType: String,
         hostname: String,
         port: Int
-    ) throws {
+    ) async throws {
         let result = HostKeyStore.shared.verify(
             keyData: keyData,
             keyType: keyType,
@@ -43,7 +42,7 @@ internal enum HostKeyVerifier {
 
         case .unknown(let fingerprint, let keyType):
             logger.info("Unknown host key for [\(hostname)]:\(port), prompting user")
-            let accepted = promptUnknownHost(
+            let accepted = await promptUnknownHost(
                 hostname: hostname,
                 port: port,
                 fingerprint: fingerprint,
@@ -62,7 +61,7 @@ internal enum HostKeyVerifier {
 
         case .mismatch(let expected, let actual):
             logger.warning("Host key mismatch for [\(hostname)]:\(port)")
-            let accepted = promptHostKeyMismatch(
+            let accepted = await promptHostKeyMismatch(
                 hostname: hostname,
                 port: port,
                 expected: expected,
@@ -83,17 +82,14 @@ internal enum HostKeyVerifier {
 
     // MARK: - UI Prompts
 
-    /// Show a dialog asking the user whether to trust an unknown host
-    /// Blocks the calling thread until the user responds.
+    /// Show a dialog asking the user whether to trust an unknown host.
+    /// Suspends until the user responds, without blocking any thread.
     private static func promptUnknownHost(
         hostname: String,
         port: Int,
         fingerprint: String,
         keyType: String
-    ) -> Bool {
-        let semaphore = DispatchSemaphore(value: 0)
-        var accepted = false
-
+    ) async -> Bool {
         let hostDisplay = "[\(hostname)]:\(port)"
         let title = String(localized: "Unknown SSH Host")
         let message = String(localized: """
@@ -105,7 +101,7 @@ internal enum HostKeyVerifier {
             Are you sure you want to continue connecting?
             """)
 
-        DispatchQueue.main.async {
+        return await MainActor.run {
             let alert = NSAlert()
             alert.messageText = title
             alert.informativeText = message
@@ -114,25 +110,18 @@ internal enum HostKeyVerifier {
             alert.addButton(withTitle: String(localized: "Cancel"))
 
             let response = alert.runModal()
-            accepted = (response == .alertFirstButtonReturn)
-            semaphore.signal()
+            return response == .alertFirstButtonReturn
         }
-
-        semaphore.wait()
-        return accepted
     }
 
-    /// Show a warning dialog about a changed host key (potential MITM attack)
-    /// Blocks the calling thread until the user responds.
+    /// Show a warning dialog about a changed host key (potential MITM attack).
+    /// Suspends until the user responds, without blocking any thread.
     private static func promptHostKeyMismatch(
         hostname: String,
         port: Int,
         expected: String,
         actual: String
-    ) -> Bool {
-        let semaphore = DispatchSemaphore(value: 0)
-        var accepted = false
-
+    ) async -> Bool {
         let hostDisplay = "[\(hostname)]:\(port)"
         let title = String(localized: "SSH Host Key Changed")
         let message = String(localized: """
@@ -144,7 +133,7 @@ internal enum HostKeyVerifier {
             Current fingerprint: \(actual)
             """)
 
-        DispatchQueue.main.async {
+        return await MainActor.run {
             let alert = NSAlert()
             alert.messageText = title
             alert.informativeText = message
@@ -157,11 +146,7 @@ internal enum HostKeyVerifier {
             alert.buttons[0].keyEquivalent = ""
 
             let response = alert.runModal()
-            accepted = (response == .alertFirstButtonReturn)
-            semaphore.signal()
+            return response == .alertFirstButtonReturn
         }
-
-        semaphore.wait()
-        return accepted
     }
 }
diff --git a/TablePro/Core/SSH/LibSSH2TunnelFactory.swift b/TablePro/Core/SSH/LibSSH2TunnelFactory.swift
@@ -38,7 +38,7 @@ internal enum LibSSH2TunnelFactory {
         remoteHost: String,
         remotePort: Int,
         localPort: Int
-    ) throws -> LibSSH2Tunnel {
+    ) async throws -> LibSSH2Tunnel {
         _ = initialized
 
         // Connect to the SSH server (or first jump host if jumps are configured)
@@ -63,7 +63,7 @@ internal enum LibSSH2TunnelFactory {
 
             do {
                 // Verify host key
-                try verifyHostKey(session: session, hostname: targetHost, port: targetPort)
+                try await verifyHostKey(session: session, hostname: targetHost, port: targetPort)
 
                 // Authenticate first hop
                 if let firstJump = config.jumpHosts.first {
@@ -141,7 +141,7 @@ internal enum LibSSH2TunnelFactory {
 
                         do {
                             // Verify host key for next hop
-                            try verifyHostKey(session: nextSession, hostname: nextHost, port: nextPort)
+                            try await verifyHostKey(session: nextSession, hostname: nextHost, port: nextPort)
 
                             // Authenticate next hop
                             if jumpIndex + 1 < jumps.count {
@@ -324,7 +324,7 @@ internal enum LibSSH2TunnelFactory {
         session: OpaquePointer,
         hostname: String,
         port: Int
-    ) throws {
+    ) async throws {
         var keyLength = 0
         var keyType: Int32 = 0
         guard let keyPtr = libssh2_session_hostkey(session, &keyLength, &keyType) else {
@@ -334,7 +334,7 @@ internal enum LibSSH2TunnelFactory {
         let keyData = Data(bytes: keyPtr, count: keyLength)
         let keyTypeName = HostKeyStore.keyTypeName(keyType)
 
-        try HostKeyVerifier.verify(
+        try await HostKeyVerifier.verify(
             keyData: keyData,
             keyType: keyTypeName,
             hostname: hostname,
diff --git a/TablePro/Core/SSH/SSHTunnelManager.swift b/TablePro/Core/SSH/SSHTunnelManager.swift
@@ -103,7 +103,7 @@ actor SSHTunnelManager {
         for localPort in localPortCandidates() {
             do {
                 let tunnel = try await Task.detached {
-                    try LibSSH2TunnelFactory.createTunnel(
+                    try await LibSSH2TunnelFactory.createTunnel(
                         connectionId: connectionId,
                         config: config,
                         credentials: credentials,
diff --git a/TablePro/Core/Services/Formatting/SQLFormatterService.swift b/TablePro/Core/Services/Formatting/SQLFormatterService.swift
diff --git a/TablePro/Core/Services/Query/SQLDialectProvider.swift b/TablePro/Core/Services/Query/SQLDialectProvider.swift
diff --git a/TablePro/Core/Sync/SyncCoordinator.swift b/TablePro/Core/Sync/SyncCoordinator.swift

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,7 @@ enum PluginError: LocalizedError {`
`20`	`20`	`case pluginNotInstalled(String)`
`21`	`21`	`case incompatibleWithCurrentApp(minimumRequired: String)`
`22`	`22`	`case invalidDescriptor(pluginId: String, reason: String)`
	`23`	`+ case pluginNotLoaded(String)`
`23`	`24`
`24`	`25`	`var errorDescription: String? {`
`25`	`26`	`switch self {`
`@@ -51,6 +52,8 @@ enum PluginError: LocalizedError {`
`51`	`52`	`return String(localized: "This plugin requires TablePro \(minimumRequired) or later")`
`52`	`53`	`case .invalidDescriptor(let pluginId, let reason):`
`53`	`54`	`return String(localized: "Plugin '\(pluginId)' has an invalid descriptor: \(reason)")`
	`55`	`+ case .pluginNotLoaded(let databaseType):`
	`56`	`+ return String(localized: "The \(databaseType) driver plugin is still loading. Please try again in a moment.")`
`54`	`57`	`}`
`55`	`58`	`}`
`56`	`59`	`}`