Skip to content

Commit a4fedfd

Browse files
datlechindatlechin
authored
Revert "fix: resolve critical concurrency deadlocks and main-thread blocking …" (#404)
This reverts commit f82d940.
1 parent f82d940 commit a4fedfd

File tree

11 files changed

+185
-198
lines changed

11 files changed

+185
-198
lines changed

‎TablePro/AppDelegate+ConnectionHandler.swift‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -329,17 +329,13 @@ extension AppDelegate {
329329
}
330330

331331
private func waitForConnection(timeout: Duration) async {
332-
let didResume = OSAllocatedUnfairLock(initialState: false)
333332
await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in
333+
var didResume = false
334334
var observer: NSObjectProtocol?
335335

336336
func resumeOnce() {
337-
let shouldResume = didResume.withLock { alreadyResumed -> Bool in
338-
if alreadyResumed { return false }
339-
alreadyResumed = true
340-
return true
341-
}
342-
guard shouldResume else { return }
337+
guard !didResume else { return }
338+
didResume = true
343339
if let obs = observer {
344340
NotificationCenter.default.removeObserver(obs)
345341
}

‎TablePro/ContentView.swift‎

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,12 @@ struct ContentView: View {
9292
payload: payload
9393
)
9494
}
95+
AppState.shared.isConnected = true
96+
AppState.shared.safeModeLevel = session.connection.safeModeLevel
97+
AppState.shared.editorLanguage = PluginManager.shared.editorLanguage(for: session.connection.type)
98+
AppState.shared.currentDatabaseType = session.connection.type
99+
AppState.shared.supportsDatabaseSwitching = PluginManager.shared.supportsDatabaseSwitching(
100+
for: session.connection.type)
95101
}
96102
} else {
97103
currentSession = nil
@@ -124,7 +130,20 @@ struct ContentView: View {
124130
}()
125131
guard isOurWindow else { return }
126132

127-
syncAppStateWithCurrentSession()
133+
if let session = DatabaseManager.shared.activeSessions[connectionId] {
134+
AppState.shared.isConnected = true
135+
AppState.shared.safeModeLevel = session.connection.safeModeLevel
136+
AppState.shared.editorLanguage = PluginManager.shared.editorLanguage(for: session.connection.type)
137+
AppState.shared.currentDatabaseType = session.connection.type
138+
AppState.shared.supportsDatabaseSwitching = PluginManager.shared.supportsDatabaseSwitching(
139+
for: session.connection.type)
140+
} else {
141+
AppState.shared.isConnected = false
142+
AppState.shared.safeModeLevel = .silent
143+
AppState.shared.editorLanguage = .sql
144+
AppState.shared.currentDatabaseType = nil
145+
AppState.shared.supportsDatabaseSwitching = true
146+
}
128147
}
129148
.onChange(of: sessionState?.toolbarState.safeModeLevel) { _, newLevel in
130149
if let level = newLevel {
@@ -330,7 +349,11 @@ struct ContentView: View {
330349
sessionState = nil
331350
currentSession = nil
332351
columnVisibility = .detailOnly
333-
syncAppStateWithCurrentSession()
352+
AppState.shared.isConnected = false
353+
AppState.shared.safeModeLevel = .silent
354+
AppState.shared.editorLanguage = .sql
355+
AppState.shared.currentDatabaseType = nil
356+
AppState.shared.supportsDatabaseSwitching = true
334357

335358
let tabbingId = "com.TablePro.main.\(sid.uuidString)"
336359
DispatchQueue.main.async {
@@ -356,27 +379,12 @@ struct ContentView: View {
356379
payload: payload
357380
)
358381
}
359-
}
360-
361-
/// Single authoritative source for syncing AppState fields with the current session.
362-
/// Called from `windowDidBecomeKey` (the correct trigger for per-window AppState)
363-
/// and from `handleConnectionStatusChange` on disconnect cleanup.
364-
private func syncAppStateWithCurrentSession() {
365-
let connectionId = payload?.connectionId ?? currentSession?.id
366-
if let connectionId, let session = DatabaseManager.shared.activeSessions[connectionId] {
367-
AppState.shared.isConnected = true
368-
AppState.shared.safeModeLevel = session.connection.safeModeLevel
369-
AppState.shared.editorLanguage = PluginManager.shared.editorLanguage(for: session.connection.type)
370-
AppState.shared.currentDatabaseType = session.connection.type
371-
AppState.shared.supportsDatabaseSwitching = PluginManager.shared.supportsDatabaseSwitching(
372-
for: session.connection.type)
373-
} else {
374-
AppState.shared.isConnected = false
375-
AppState.shared.safeModeLevel = .silent
376-
AppState.shared.editorLanguage = .sql
377-
AppState.shared.currentDatabaseType = nil
378-
AppState.shared.supportsDatabaseSwitching = true
379-
}
382+
AppState.shared.isConnected = true
383+
AppState.shared.safeModeLevel = newSession.connection.safeModeLevel
384+
AppState.shared.editorLanguage = PluginManager.shared.editorLanguage(for: newSession.connection.type)
385+
AppState.shared.currentDatabaseType = newSession.connection.type
386+
AppState.shared.supportsDatabaseSwitching = PluginManager.shared.supportsDatabaseSwitching(
387+
for: newSession.connection.type)
380388
}
381389

382390
// MARK: - Actions

‎TablePro/Core/Database/DatabaseDriver.swift‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -317,12 +317,13 @@ extension DatabaseDriver {
317317
enum DatabaseDriverFactory {
318318
static func createDriver(for connection: DatabaseConnection) throws -> DatabaseDriver {
319319
let pluginId = connection.type.pluginTypeId
320+
// If the plugin isn't registered yet and background loading hasn't finished,
321+
// fall back to synchronous loading for this critical code path.
322+
if PluginManager.shared.driverPlugins[pluginId] == nil,
323+
!PluginManager.shared.hasFinishedInitialLoad {
324+
PluginManager.shared.loadPendingPlugins()
325+
}
320326
guard let plugin = PluginManager.shared.driverPlugins[pluginId] else {
321-
// If background loading hasn't finished yet, throw a specific error
322-
// instead of blocking the main thread with synchronous plugin loading.
323-
if !PluginManager.shared.hasFinishedInitialLoad {
324-
throw PluginError.pluginNotLoaded(connection.type.rawValue)
325-
}
326327
if connection.type.isDownloadablePlugin {
327328
throw PluginError.pluginNotInstalled(connection.type.rawValue)
328329
}

‎TablePro/Core/Plugins/PluginError.swift‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ enum PluginError: LocalizedError {
2020
case pluginNotInstalled(String)
2121
case incompatibleWithCurrentApp(minimumRequired: String)
2222
case invalidDescriptor(pluginId: String, reason: String)
23-
case pluginNotLoaded(String)
2423

2524
var errorDescription: String? {
2625
switch self {
@@ -52,8 +51,6 @@ enum PluginError: LocalizedError {
5251
return String(localized: "This plugin requires TablePro \(minimumRequired) or later")
5352
case .invalidDescriptor(let pluginId, let reason):
5453
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.")
5754
}
5855
}
5956
}

‎TablePro/Core/Plugins/PluginManager.swift‎

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,6 @@ final class PluginManager {
142142
logger.error("User plugin \(entry.url.lastPathComponent) has outdated PluginKit v\(pluginKitVersion)")
143143
continue
144144
}
145-
146-
// Verify code signature off the main thread (disk I/O in SecStaticCodeCheckValidity)
147-
do {
148-
try verifyCodeSignatureStatic(bundle: bundle)
149-
} catch {
150-
logger.error("Code signature verification failed for \(entry.url.lastPathComponent): \(error.localizedDescription)")
151-
continue
152-
}
153145
}
154146

155147
// Heavy I/O: dynamic linker resolution, C bridge library initialization
@@ -316,8 +308,7 @@ final class PluginManager {
316308
current: pluginKitVersion
317309
)
318310
}
319-
// Code signature verification is deferred to loadBundlesOffMain()
320-
// to avoid blocking the main thread with SecStaticCodeCheckValidity disk I/O.
311+
try verifyCodeSignature(bundle: bundle)
321312
}
322313

323314
pendingPluginURLs.append((url: url, source: source))
@@ -1057,21 +1048,13 @@ final class PluginManager {
10571048
private static let signingTeamId = "D7HJ5TFYCU"
10581049

10591050
private func createSigningRequirement() -> SecRequirement? {
1060-
Self.createSigningRequirementStatic()
1061-
}
1062-
1063-
nonisolated private static func createSigningRequirementStatic() -> SecRequirement? {
10641051
var requirement: SecRequirement?
1065-
let requirementString = "anchor apple generic and certificate leaf[subject.OU] = \"\(signingTeamId)\"" as CFString
1052+
let requirementString = "anchor apple generic and certificate leaf[subject.OU] = \"\(Self.signingTeamId)\"" as CFString
10661053
SecRequirementCreateWithString(requirementString, SecCSFlags(), &requirement)
10671054
return requirement
10681055
}
10691056

10701057
private func verifyCodeSignature(bundle: Bundle) throws {
1071-
try Self.verifyCodeSignatureStatic(bundle: bundle)
1072-
}
1073-
1074-
nonisolated private static func verifyCodeSignatureStatic(bundle: Bundle) throws {
10751058
var staticCode: SecStaticCode?
10761059
let createStatus = SecStaticCodeCreateWithPath(
10771060
bundle.bundleURL as CFURL,
@@ -1081,11 +1064,11 @@ final class PluginManager {
10811064

10821065
guard createStatus == errSecSuccess, let code = staticCode else {
10831066
throw PluginError.signatureInvalid(
1084-
detail: describeOSStatus(createStatus)
1067+
detail: Self.describeOSStatus(createStatus)
10851068
)
10861069
}
10871070

1088-
let requirement = createSigningRequirementStatic()
1071+
let requirement = createSigningRequirement()
10891072

10901073
let checkStatus = SecStaticCodeCheckValidity(
10911074
code,
@@ -1095,12 +1078,12 @@ final class PluginManager {
10951078

10961079
guard checkStatus == errSecSuccess else {
10971080
throw PluginError.signatureInvalid(
1098-
detail: describeOSStatus(checkStatus)
1081+
detail: Self.describeOSStatus(checkStatus)
10991082
)
11001083
}
11011084
}
11021085

1103-
nonisolated private static func describeOSStatus(_ status: OSStatus) -> String {
1086+
private static func describeOSStatus(_ status: OSStatus) -> String {
11041087
switch status {
11051088
case -67_062: return "bundle is not signed"
11061089
case -67_061: return "code signature is invalid"

‎TablePro/Core/SSH/HostKeyVerifier.swift‎

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ internal enum HostKeyVerifier {
1515
private static let logger = Logger(subsystem: "com.TablePro", category: "HostKeyVerifier")
1616

1717
/// Verify the host key, prompting the user if needed.
18-
/// Uses `withCheckedContinuation` to await UI prompts without blocking the cooperative thread pool.
18+
/// This method blocks the calling thread while showing UI prompts.
19+
/// Must be called from a background thread.
1920
/// - Parameters:
2021
/// - keyData: The raw host key bytes from the SSH session
2122
/// - keyType: The key type string (e.g. "ssh-rsa", "ssh-ed25519")
@@ -27,7 +28,7 @@ internal enum HostKeyVerifier {
2728
keyType: String,
2829
hostname: String,
2930
port: Int
30-
) async throws {
31+
) throws {
3132
let result = HostKeyStore.shared.verify(
3233
keyData: keyData,
3334
keyType: keyType,
@@ -42,7 +43,7 @@ internal enum HostKeyVerifier {
4243

4344
case .unknown(let fingerprint, let keyType):
4445
logger.info("Unknown host key for [\(hostname)]:\(port), prompting user")
45-
let accepted = await promptUnknownHost(
46+
let accepted = promptUnknownHost(
4647
hostname: hostname,
4748
port: port,
4849
fingerprint: fingerprint,
@@ -61,7 +62,7 @@ internal enum HostKeyVerifier {
6162

6263
case .mismatch(let expected, let actual):
6364
logger.warning("Host key mismatch for [\(hostname)]:\(port)")
64-
let accepted = await promptHostKeyMismatch(
65+
let accepted = promptHostKeyMismatch(
6566
hostname: hostname,
6667
port: port,
6768
expected: expected,
@@ -82,14 +83,17 @@ internal enum HostKeyVerifier {
8283

8384
// MARK: - UI Prompts
8485

85-
/// Show a dialog asking the user whether to trust an unknown host.
86-
/// Suspends until the user responds, without blocking any thread.
86+
/// Show a dialog asking the user whether to trust an unknown host
87+
/// Blocks the calling thread until the user responds.
8788
private static func promptUnknownHost(
8889
hostname: String,
8990
port: Int,
9091
fingerprint: String,
9192
keyType: String
92-
) async -> Bool {
93+
) -> Bool {
94+
let semaphore = DispatchSemaphore(value: 0)
95+
var accepted = false
96+
9397
let hostDisplay = "[\(hostname)]:\(port)"
9498
let title = String(localized: "Unknown SSH Host")
9599
let message = String(localized: """
@@ -101,7 +105,7 @@ internal enum HostKeyVerifier {
101105
Are you sure you want to continue connecting?
102106
""")
103107

104-
return await MainActor.run {
108+
DispatchQueue.main.async {
105109
let alert = NSAlert()
106110
alert.messageText = title
107111
alert.informativeText = message
@@ -110,18 +114,25 @@ internal enum HostKeyVerifier {
110114
alert.addButton(withTitle: String(localized: "Cancel"))
111115

112116
let response = alert.runModal()
113-
return response == .alertFirstButtonReturn
117+
accepted = (response == .alertFirstButtonReturn)
118+
semaphore.signal()
114119
}
120+
121+
semaphore.wait()
122+
return accepted
115123
}
116124

117-
/// Show a warning dialog about a changed host key (potential MITM attack).
118-
/// Suspends until the user responds, without blocking any thread.
125+
/// Show a warning dialog about a changed host key (potential MITM attack)
126+
/// Blocks the calling thread until the user responds.
119127
private static func promptHostKeyMismatch(
120128
hostname: String,
121129
port: Int,
122130
expected: String,
123131
actual: String
124-
) async -> Bool {
132+
) -> Bool {
133+
let semaphore = DispatchSemaphore(value: 0)
134+
var accepted = false
135+
125136
let hostDisplay = "[\(hostname)]:\(port)"
126137
let title = String(localized: "SSH Host Key Changed")
127138
let message = String(localized: """
@@ -133,7 +144,7 @@ internal enum HostKeyVerifier {
133144
Current fingerprint: \(actual)
134145
""")
135146

136-
return await MainActor.run {
147+
DispatchQueue.main.async {
137148
let alert = NSAlert()
138149
alert.messageText = title
139150
alert.informativeText = message
@@ -146,7 +157,11 @@ internal enum HostKeyVerifier {
146157
alert.buttons[0].keyEquivalent = ""
147158

148159
let response = alert.runModal()
149-
return response == .alertFirstButtonReturn
160+
accepted = (response == .alertFirstButtonReturn)
161+
semaphore.signal()
150162
}
163+
164+
semaphore.wait()
165+
return accepted
151166
}
152167
}

‎TablePro/Core/SSH/LibSSH2TunnelFactory.swift‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ internal enum LibSSH2TunnelFactory {
3838
remoteHost: String,
3939
remotePort: Int,
4040
localPort: Int
41-
) async throws -> LibSSH2Tunnel {
41+
) throws -> LibSSH2Tunnel {
4242
_ = initialized
4343

4444
// Connect to the SSH server (or first jump host if jumps are configured)
@@ -63,7 +63,7 @@ internal enum LibSSH2TunnelFactory {
6363

6464
do {
6565
// Verify host key
66-
try await verifyHostKey(session: session, hostname: targetHost, port: targetPort)
66+
try verifyHostKey(session: session, hostname: targetHost, port: targetPort)
6767

6868
// Authenticate first hop
6969
if let firstJump = config.jumpHosts.first {
@@ -141,7 +141,7 @@ internal enum LibSSH2TunnelFactory {
141141

142142
do {
143143
// Verify host key for next hop
144-
try await verifyHostKey(session: nextSession, hostname: nextHost, port: nextPort)
144+
try verifyHostKey(session: nextSession, hostname: nextHost, port: nextPort)
145145

146146
// Authenticate next hop
147147
if jumpIndex + 1 < jumps.count {
@@ -324,7 +324,7 @@ internal enum LibSSH2TunnelFactory {
324324
session: OpaquePointer,
325325
hostname: String,
326326
port: Int
327-
) async throws {
327+
) throws {
328328
var keyLength = 0
329329
var keyType: Int32 = 0
330330
guard let keyPtr = libssh2_session_hostkey(session, &keyLength, &keyType) else {
@@ -334,7 +334,7 @@ internal enum LibSSH2TunnelFactory {
334334
let keyData = Data(bytes: keyPtr, count: keyLength)
335335
let keyTypeName = HostKeyStore.keyTypeName(keyType)
336336

337-
try await HostKeyVerifier.verify(
337+
try HostKeyVerifier.verify(
338338
keyData: keyData,
339339
keyType: keyTypeName,
340340
hostname: hostname,

‎TablePro/Core/SSH/SSHTunnelManager.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ actor SSHTunnelManager {
103103
for localPort in localPortCandidates() {
104104
do {
105105
let tunnel = try await Task.detached {
106-
try await LibSSH2TunnelFactory.createTunnel(
106+
try LibSSH2TunnelFactory.createTunnel(
107107
connectionId: connectionId,
108108
config: config,
109109
credentials: credentials,

0 commit comments

Comments
 (0)