Skip to content

Commit d4ca88e

Browse files
authored
Merge pull request #218 from datlechin/fix/resource-cleanup-audit
fix: clean up leaked observers, untracked tasks, and misleading log
2 parents c8c2216 + 1156766 commit d4ca88e

3 files changed

Lines changed: 26 additions & 9 deletions

File tree

TablePro/Core/Database/DatabaseManager.swift

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ final class DatabaseManager {
3434
/// Separate from the main driver so pings never queue behind long-running user queries.
3535
private var pingDrivers: [UUID: DatabaseDriver] = [:]
3636

37+
private var metadataCreationTasks: [UUID: Task<Void, Never>] = [:]
38+
3739
/// Current session (computed from currentSessionId)
3840
var currentSession: ConnectionSession? {
3941
guard let sessionId = currentSessionId else { return nil }
@@ -204,8 +206,9 @@ final class DatabaseManager {
204206
let metaConnection = effectiveConnection
205207
let metaConnectionId = connection.id
206208
let metaTimeout = AppSettingsManager.shared.general.queryTimeoutSeconds
207-
Task { [weak self] in
209+
metadataCreationTasks[metaConnectionId] = Task { [weak self] in
208210
guard let self else { return }
211+
defer { self.metadataCreationTasks.removeValue(forKey: metaConnectionId) }
209212
do {
210213
let metaDriver = try DatabaseDriverFactory.createDriver(for: metaConnection)
211214
try await metaDriver.connect()
@@ -269,6 +272,10 @@ final class DatabaseManager {
269272
try? await SSHTunnelManager.shared.closeTunnel(connectionId: session.connection.id)
270273
}
271274

275+
// Cancel any in-flight metadata driver creation
276+
metadataCreationTasks[sessionId]?.cancel()
277+
metadataCreationTasks.removeValue(forKey: sessionId)
278+
272279
// Stop health monitoring
273280
await stopHealthMonitor(for: sessionId)
274281

@@ -301,6 +308,9 @@ final class DatabaseManager {
301308
await stopHealthMonitor(for: sessionId)
302309
}
303310

311+
for task in metadataCreationTasks.values { task.cancel() }
312+
metadataCreationTasks.removeAll()
313+
304314
let sessionIds = Array(activeSessions.keys)
305315
for sessionId in sessionIds {
306316
await disconnectSession(sessionId)
@@ -539,7 +549,7 @@ final class DatabaseManager {
539549
}
540550
case .failed:
541551
Self.logger.error(
542-
"Health monitoring failed for session \(id) after 3 retries")
552+
"Health monitoring failed for session \(id)")
543553
self.updateSession(id) { session in
544554
session.status = .error(String(localized: "Connection lost"))
545555
session.clearCachedData()
@@ -674,8 +684,9 @@ final class DatabaseManager {
674684
let metaTimeout = AppSettingsManager.shared.general.queryTimeoutSeconds
675685
let startupCmds = session.connection.startupCommands
676686
let connName = session.connection.name
677-
Task { [weak self] in
687+
metadataCreationTasks[metaConnectionId] = Task { [weak self] in
678688
guard let self else { return }
689+
defer { self.metadataCreationTasks.removeValue(forKey: metaConnectionId) }
679690
do {
680691
let metaDriver = try DatabaseDriverFactory.createDriver(for: metaConnection)
681692
try await metaDriver.connect()

TablePro/Views/Main/Extensions/MainContentCoordinator+URLFilter.swift

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
import Foundation
77

88
extension MainContentCoordinator {
9-
func setupURLNotificationObservers() {
9+
func setupURLNotificationObservers() -> [NSObjectProtocol] {
1010
let connId = connectionId
11-
NotificationCenter.default.addObserver(
11+
let observer1 = NotificationCenter.default.addObserver(
1212
forName: .applyURLFilter,
1313
object: nil,
1414
queue: .main
@@ -17,7 +17,6 @@ extension MainContentCoordinator {
1717
let targetId = userInfo["connectionId"] as? UUID,
1818
targetId == connId else { return }
1919

20-
// Extract Sendable values before crossing isolation boundary
2120
let condition = userInfo["condition"] as? String
2221
let column = userInfo["column"] as? String
2322
let operation = userInfo["operation"] as? String
@@ -30,7 +29,7 @@ extension MainContentCoordinator {
3029
}
3130
}
3231

33-
NotificationCenter.default.addObserver(
32+
let observer2 = NotificationCenter.default.addObserver(
3433
forName: .switchSchemaFromURL,
3534
object: nil,
3635
queue: .main
@@ -42,14 +41,16 @@ extension MainContentCoordinator {
4241

4342
Task { @MainActor [weak self] in
4443
guard let self else { return }
45-
44+
4645
if self.connection.type == .postgresql {
4746
await self.switchSchema(to: schema)
4847
} else {
4948
await self.switchDatabase(to: schema)
5049
}
5150
}
5251
}
52+
53+
return [observer1, observer2]
5354
}
5455

5556
private func applyURLFilterValues(

TablePro/Views/Main/MainContentCoordinator.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ final class MainContentCoordinator {
7777
@ObservationIgnored private var changeManagerUpdateTask: Task<Void, Never>?
7878
@ObservationIgnored private var activeSortTasks: [UUID: Task<Void, Never>] = [:]
7979
@ObservationIgnored private var terminationObserver: NSObjectProtocol?
80+
@ObservationIgnored private var urlFilterObservers: [NSObjectProtocol] = []
8081

8182
/// Set during handleTabChange to suppress redundant onChange(of: resultColumns) reconfiguration
8283
@ObservationIgnored internal var isHandlingTabSwitch = false
@@ -156,7 +157,7 @@ final class MainContentCoordinator {
156157

157158
self.schemaProvider = SchemaProviderRegistry.shared.getOrCreate(for: connection.id)
158159
SchemaProviderRegistry.shared.retain(for: connection.id)
159-
setupURLNotificationObservers()
160+
urlFilterObservers = setupURLNotificationObservers()
160161

161162
// Synchronous save at quit time. NotificationCenter with queue: .main
162163
// delivers the closure on the main thread, satisfying assumeIsolated's
@@ -195,6 +196,10 @@ final class MainContentCoordinator {
195196
/// synchronously on MainActor so we don't depend on deinit + Task scheduling.
196197
func teardown() {
197198
_didTeardown.withLock { $0 = true }
199+
for observer in urlFilterObservers {
200+
NotificationCenter.default.removeObserver(observer)
201+
}
202+
urlFilterObservers.removeAll()
198203
if let observer = terminationObserver {
199204
NotificationCenter.default.removeObserver(observer)
200205
terminationObserver = nil

0 commit comments

Comments
 (0)