From 38e46b8afe52df678a65e15114578850386a8977 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 23 Mar 2026 21:07:01 +0700 Subject: [PATCH 01/10] fix: reduce memory retention after closing tabs - Clear changeManager state and pluginDriver reference in teardown - Cancel redisDatabaseSwitchTask in teardown - Clear cachedTableColumnTypes/Names, tableMetadata, filterState in teardown - Release editor closures and heavy state (tree-sitter, highlighter) on destroy - Add releaseHeavyState() to TextViewController for early resource cleanup - Make InMemoryRowProvider.rowBuffer weak with safe fallback - Add releaseData() to InMemoryRowProvider for explicit cleanup - Clear tabProviderCache, sortCache, cachedChangeManager in onTeardown - Hint malloc to return freed pages after disconnect - Add deinit logging for RowBuffer and QueryTabManager --- .../Controller/TextViewController.swift | 23 +++++++- TablePro/Models/Query/QueryTab.swift | 11 ++++ TablePro/Models/Query/RowProvider.swift | 30 +++++++--- TablePro/Resources/Localizable.xcstrings | 3 + .../Views/Editor/SQLEditorCoordinator.swift | 15 +++++ TablePro/Views/Editor/SQLEditorView.swift | 1 + .../Main/Child/MainEditorContentView.swift | 5 ++ .../Views/Main/MainContentCoordinator.swift | 34 ++++++++++++ TablePro/Views/Main/MainContentView.swift | 11 +++- TablePro/Views/Results/DataGridView.swift | 55 +++++++++++++++++++ 10 files changed, 178 insertions(+), 10 deletions(-) diff --git a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/Controller/TextViewController.swift b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/Controller/TextViewController.swift index fb95c81c..2b9258ad 100644 --- a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/Controller/TextViewController.swift +++ b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/Controller/TextViewController.swift @@ -290,9 +290,30 @@ public class TextViewController: NSViewController { self.gutterView.setNeedsDisplay(self.gutterView.frame) } + /// Release heavy resources (tree-sitter, highlighter, text storage) early, + /// without waiting for deinit. Call when the editor is no longer visible but + /// SwiftUI may keep the controller alive in @State. + public func releaseHeavyState() { + if let highlighter { + textView?.removeStorageDelegate(highlighter) + } + highlighter = nil + treeSitterClient = nil + highlightProviders.removeAll() + textCoordinators.values().forEach { $0.destroy() } + textCoordinators.removeAll() + cancellables.forEach { $0.cancel() } + cancellables.removeAll() + if let localEventMonitor { + NSEvent.removeMonitor(localEventMonitor) + } + localEventMonitor = nil + textView?.setText("") + } + deinit { if let highlighter { - textView.removeStorageDelegate(highlighter) + textView?.removeStorageDelegate(highlighter) } highlighter = nil highlightProviders.removeAll() diff --git a/TablePro/Models/Query/QueryTab.swift b/TablePro/Models/Query/QueryTab.swift index 5963cac2..89fe602c 100644 --- a/TablePro/Models/Query/QueryTab.swift +++ b/TablePro/Models/Query/QueryTab.swift @@ -7,6 +7,7 @@ import Foundation import Observation +import os import TableProPluginKit /// Type of tab @@ -269,6 +270,11 @@ final class RowBuffer { self.rows = newRows isEvicted = false } + + deinit { + Logger(subsystem: "com.TablePro", category: "RowBuffer") + .debug("RowBuffer deallocated — columns: \(self.columns.count), evicted: \(self.isEvicted)") + } } /// Represents a single tab (query or table) @@ -676,4 +682,9 @@ final class QueryTabManager { tabs[index] = tab } } + + deinit { + Logger(subsystem: "com.TablePro", category: "QueryTabManager") + .debug("QueryTabManager deallocated") + } } diff --git a/TablePro/Models/Query/RowProvider.swift b/TablePro/Models/Query/RowProvider.swift index ed91e419..221e5054 100644 --- a/TablePro/Models/Query/RowProvider.swift +++ b/TablePro/Models/Query/RowProvider.swift @@ -66,7 +66,12 @@ final class TableRowData { /// Direct-access methods `value(atRow:column:)` and `rowValues(at:)` avoid /// heap allocations by reading straight from the source `[String?]` array. final class InMemoryRowProvider: RowProvider { - private let rowBuffer: RowBuffer + private weak var rowBuffer: RowBuffer? + /// Strong reference only when the provider created its own buffer (convenience init). + /// External buffers are owned by QueryTab, so we hold them weakly. + private var ownedBuffer: RowBuffer? + private static let emptyBuffer = RowBuffer() + private var safeBuffer: RowBuffer { rowBuffer ?? Self.emptyBuffer } private var sortIndices: [Int]? private var appendedRows: [[String?]] = [] private(set) var columns: [String] @@ -86,7 +91,7 @@ final class InMemoryRowProvider: RowProvider { /// Number of rows coming from the buffer (respecting sort indices count when present) private var bufferRowCount: Int { - sortIndices?.count ?? rowBuffer.rows.count + sortIndices?.count ?? safeBuffer.rows.count } init( @@ -130,6 +135,7 @@ final class InMemoryRowProvider: RowProvider { columnEnumValues: columnEnumValues, columnNullable: columnNullable ) + ownedBuffer = buffer } func fetchRows(offset: Int, limit: Int) -> [TableRowData] { @@ -157,7 +163,7 @@ final class InMemoryRowProvider: RowProvider { guard rowIndex < totalRowCount else { return } let sourceIndex = resolveSourceIndex(rowIndex) if let bufferIdx = sourceIndex.bufferIndex { - rowBuffer.rows[bufferIdx][columnIndex] = value + safeBuffer.rows[bufferIdx][columnIndex] = value displayCache.removeValue(forKey: bufferIdx) } else if let appendedIdx = sourceIndex.appendedIndex { appendedRows[appendedIdx][columnIndex] = value @@ -215,9 +221,17 @@ final class InMemoryRowProvider: RowProvider { displayCache.removeAll() } + /// Release cached data to free memory when this provider is no longer active. + func releaseData() { + displayCache.removeAll() + appendedRows.removeAll() + sortIndices = nil + ownedBuffer = nil + } + /// Update rows by replacing the buffer contents and clearing appended rows func updateRows(_ newRows: [[String?]]) { - rowBuffer.rows = newRows + safeBuffer.rows = newRows appendedRows.removeAll() sortIndices = nil displayCache.removeAll() @@ -242,7 +256,7 @@ final class InMemoryRowProvider: RowProvider { } else { if let sorted = sortIndices { let bufferIdx = sorted[index] - rowBuffer.rows.remove(at: bufferIdx) + safeBuffer.rows.remove(at: bufferIdx) var newIndices = sorted newIndices.remove(at: index) for i in newIndices.indices where newIndices[i] > bufferIdx { @@ -250,7 +264,7 @@ final class InMemoryRowProvider: RowProvider { } sortIndices = newIndices } else { - rowBuffer.rows.remove(at: index) + safeBuffer.rows.remove(at: index) } } displayCache.removeAll() @@ -297,9 +311,9 @@ final class InMemoryRowProvider: RowProvider { return appendedRows[displayIndex - bCount] } if let sorted = sortIndices { - return rowBuffer.rows[sorted[displayIndex]] + return safeBuffer.rows[sorted[displayIndex]] } - return rowBuffer.rows[displayIndex] + return safeBuffer.rows[displayIndex] } } diff --git a/TablePro/Resources/Localizable.xcstrings b/TablePro/Resources/Localizable.xcstrings index 569d2ca3..43d9993d 100644 --- a/TablePro/Resources/Localizable.xcstrings +++ b/TablePro/Resources/Localizable.xcstrings @@ -27181,6 +27181,9 @@ } } } + }, + "SSH Connection Test Failed" : { + }, "SSH connection timed out" : { "localizations" : { diff --git a/TablePro/Views/Editor/SQLEditorCoordinator.swift b/TablePro/Views/Editor/SQLEditorCoordinator.swift index 53cb569d..e19cfdb6 100644 --- a/TablePro/Views/Editor/SQLEditorCoordinator.swift +++ b/TablePro/Views/Editor/SQLEditorCoordinator.swift @@ -146,7 +146,22 @@ final class SQLEditorCoordinator: TextViewCoordinator { inlineSuggestionManager?.uninstall() inlineSuggestionManager = nil + // Release closure captures to break potential retain cycles + onCloseTab = nil + onExecuteQuery = nil + onAIExplain = nil + onAIOptimize = nil + onSaveAsFavorite = nil + schemaProvider = nil + contextMenu = nil + vimEngine = nil + vimCursorManager = nil + + // Release editor controller heavy state + controller?.releaseHeavyState() + EditorEventRouter.shared.unregister(self) + Self.logger.debug("SQLEditorCoordinator destroyed") cleanupMonitors() } diff --git a/TablePro/Views/Editor/SQLEditorView.swift b/TablePro/Views/Editor/SQLEditorView.swift index 07bdd4ac..c1e72793 100644 --- a/TablePro/Views/Editor/SQLEditorView.swift +++ b/TablePro/Views/Editor/SQLEditorView.swift @@ -126,6 +126,7 @@ struct SQLEditorView: View { .onDisappear { teardownFavoritesObserver() coordinator.destroy() + completionAdapter = nil } .onChange(of: coordinator.vimMode) { _, newMode in vimMode = newMode diff --git a/TablePro/Views/Main/Child/MainEditorContentView.swift b/TablePro/Views/Main/Child/MainEditorContentView.swift index 32a4910e..a6aa331e 100644 --- a/TablePro/Views/Main/Child/MainEditorContentView.swift +++ b/TablePro/Views/Main/Child/MainEditorContentView.swift @@ -147,6 +147,11 @@ struct MainEditorContentView: View { if let tab = tabManager.selectedTab { cacheRowProvider(for: tab) } + coordinator.onTeardown = { [self] in + tabProviderCache.removeAll() + sortCache.removeAll() + cachedChangeManager = nil + } } .onChange(of: tabManager.selectedTab?.resultVersion) { _, newVersion in guard let tab = tabManager.selectedTab, newVersion != nil else { return } diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index bb42b5b4..bb1a1140 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -46,6 +46,10 @@ enum ActiveSheet: Identifiable { final class MainContentCoordinator { static let logger = Logger(subsystem: "com.TablePro", category: "MainContentCoordinator") + /// Posted during teardown so DataGridView coordinators can release cell views. + /// Object is the connection UUID. + static let teardownNotification = Notification.Name("MainContentCoordinator.teardown") + // MARK: - Dependencies let connection: DatabaseConnection @@ -125,6 +129,9 @@ final class MainContentCoordinator { /// (e.g. save-then-close). Set before calling `saveChanges`, resumed by `executeCommitStatements`. @ObservationIgnored internal var saveCompletionContinuation: CheckedContinuation? + /// Called during teardown to let the view layer release cached row providers and sort data. + @ObservationIgnored var onTeardown: (() -> Void)? + /// True while a database switch is in progress. Guards against /// side-effect window creation during the switch cascade. var isSwitchingDatabase = false @@ -334,6 +341,7 @@ final class MainContentCoordinator { /// synchronously on MainActor so we don't depend on deinit + Task scheduling. func teardown() { _didTeardown.withLock { $0 = true } + unregisterFromPersistence() for observer in urlFilterObservers { NotificationCenter.default.removeObserver(observer) @@ -351,18 +359,44 @@ final class MainContentCoordinator { currentQueryTask = nil changeManagerUpdateTask?.cancel() changeManagerUpdateTask = nil + redisDatabaseSwitchTask?.cancel() + redisDatabaseSwitchTask = nil for task in activeSortTasks.values { task.cancel() } activeSortTasks.removeAll() + // Let the view layer release cached row providers before we drop RowBuffers. + // Called synchronously here because SwiftUI onChange handlers don't fire + // reliably on disappearing views. + onTeardown?() + onTeardown = nil + + // Notify DataGridView coordinators to release NSTableView cell views + NotificationCenter.default.post( + name: Self.teardownNotification, + object: connection.id + ) + // Release heavy data so memory drops even if SwiftUI delays deallocation for tab in tabManager.tabs { tab.rowBuffer.evict() } querySortCache.removeAll() + cachedTableColumnTypes.removeAll() + cachedTableColumnNames.removeAll() tabManager.tabs.removeAll() tabManager.selectedTabId = nil + // Release change manager state — pluginDriver holds a strong reference + // to the entire database driver which prevents deallocation + changeManager.clearChanges() + changeManager.pluginDriver = nil + + // Release metadata and filter state + tableMetadata = nil + filterStateManager.filters.removeAll() + filterStateManager.appliedFilters.removeAll() + SchemaProviderRegistry.shared.release(for: connection.id) SchemaProviderRegistry.shared.purgeUnused() } diff --git a/TablePro/Views/Main/MainContentView.swift b/TablePro/Views/Main/MainContentView.swift index 730a18b0..4cce7909 100644 --- a/TablePro/Views/Main/MainContentView.swift +++ b/TablePro/Views/Main/MainContentView.swift @@ -272,7 +272,11 @@ struct MainContentView: View { // If no more windows for this connection, disconnect. // Tab state is NOT cleared here — it's preserved for next reconnect. // Only handleTabsChange(count=0) clears state (user explicitly closed all tabs). - guard !WindowLifecycleMonitor.shared.hasWindows(for: connectionId) else { return } + guard !WindowLifecycleMonitor.shared.hasWindows(for: connectionId) else { + // Hint malloc to return freed pages to the OS + malloc_zone_pressure_relief(nil, 0) + return + } let hasVisibleWindow = NSApp.windows.contains { window in window.isVisible && (window.subtitle == connectionName @@ -281,6 +285,11 @@ struct MainContentView: View { if !hasVisibleWindow { await DatabaseManager.shared.disconnectSession(connectionId) } + + // Give SwiftUI/AppKit time to deallocate view hierarchies, + // then hint malloc to return freed pages to the OS + try? await Task.sleep(for: .seconds(2)) + malloc_zone_pressure_relief(nil, 0) } } .onChange(of: pendingChangeTrigger) { diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index f63ff6aa..1497fd28 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -166,6 +166,9 @@ struct DataGridView: NSViewRepresentable { scrollView.documentView = tableView context.coordinator.tableView = tableView + if let connectionId { + context.coordinator.observeTeardown(connectionId: connectionId) + } return scrollView } @@ -632,6 +635,7 @@ struct DataGridView: NSViewRepresentable { NotificationCenter.default.removeObserver(observer) coordinator.themeObserver = nil } + coordinator.rowProvider = InMemoryRowProvider(rows: [], columns: []) } func makeCoordinator() -> TableViewCoordinator { @@ -839,6 +843,54 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData } } + /// Subscribe to coordinator teardown to release NSTableView cell views. + func observeTeardown(connectionId: UUID) { + teardownObserver = NotificationCenter.default.addObserver( + forName: MainContentCoordinator.teardownNotification, + object: connectionId, + queue: nil + ) { [weak self] _ in + guard let self else { return } + MainActor.assumeIsolated { + self.releaseData() + } + } + } + + /// Release all data and cell views from the NSTableView. + /// Called during coordinator teardown to free memory while SwiftUI holds the view. + private func releaseData() { + overlayEditor?.dismiss(commit: false) + rowProvider = InMemoryRowProvider(rows: [], columns: []) + rowVisualStateCache.removeAll() + cachedRowCount = 0 + cachedColumnCount = 0 + // Remove columns and reload to release cell views + if let tableView { + while let col = tableView.tableColumns.last { + tableView.removeTableColumn(col) + } + tableView.reloadData() + } + // Release closures + onRefresh = nil + onCellEdit = nil + onDeleteRows = nil + onCopyRows = nil + onPasteRows = nil + onUndo = nil + onRedo = nil + onSort = nil + onAddRow = nil + onUndoInsert = nil + onFilterColumn = nil + onHideColumn = nil + onNavigateFK = nil + getVisualState = nil + } + + private var teardownObserver: NSObjectProtocol? + deinit { if let observer = settingsObserver { NotificationCenter.default.removeObserver(observer) @@ -846,6 +898,9 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData if let observer = themeObserver { NotificationCenter.default.removeObserver(observer) } + if let observer = teardownObserver { + NotificationCenter.default.removeObserver(observer) + } } func updateCache() { From e269dda3392977326de49af68709fcc64d3a1207 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 24 Mar 2026 11:35:11 +0700 Subject: [PATCH 02/10] fix: set isKeyWindow when WindowAccessor captures window WindowAccessor.viewDidMoveToWindow fires after didBecomeKeyNotification, so isKeyWindow was never set to true. This blocked sidebar table clicks (guarded by isKeyWindow) and caused new tabs to open as separate windows instead of attaching to the connection's tab group. --- TablePro/Views/Main/MainContentView.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/TablePro/Views/Main/MainContentView.swift b/TablePro/Views/Main/MainContentView.swift index 39aac11c..90d66f6e 100644 --- a/TablePro/Views/Main/MainContentView.swift +++ b/TablePro/Views/Main/MainContentView.swift @@ -600,6 +600,7 @@ struct MainContentView: View { isPreview: isPreview ) viewWindow = window + isKeyWindow = window.isKeyWindow // Update command actions window reference now that it's available commandActions?.window = window From cb7ecb37d78226cb1473d1f4ade2c1a5fc7efd0e Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 24 Mar 2026 11:37:16 +0700 Subject: [PATCH 03/10] fix: explicitly attach new windows to existing tab group openWindow creates the window before tabbingIdentifier is set, so macOS automatic tabbing doesn't group them. Use addTabbedWindow to explicitly attach new windows to the connection's existing tab group. --- TablePro/AppDelegate+WindowConfig.swift | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/TablePro/AppDelegate+WindowConfig.swift b/TablePro/AppDelegate+WindowConfig.swift index e575a6b9..29198fda 100644 --- a/TablePro/AppDelegate+WindowConfig.swift +++ b/TablePro/AppDelegate+WindowConfig.swift @@ -239,15 +239,25 @@ extension AppDelegate { let existingIdentifier = NSApp.windows .first { $0 !== window && isMainWindow($0) && $0.isVisible }? .tabbingIdentifier - window.tabbingIdentifier = TabbingIdentifierResolver.resolve( + let resolvedIdentifier = TabbingIdentifierResolver.resolve( pendingConnectionId: pendingId, existingIdentifier: existingIdentifier ) + window.tabbingIdentifier = resolvedIdentifier configuredWindows.insert(windowId) if !NSWindow.allowsAutomaticWindowTabbing { NSWindow.allowsAutomaticWindowTabbing = true } + + // Explicitly attach to existing tab group — automatic tabbing + // doesn't work when tabbingIdentifier is set after window creation. + if let existingWindow = NSApp.windows.first(where: { + $0 !== window && isMainWindow($0) && $0.isVisible + && $0.tabbingIdentifier == resolvedIdentifier + }) { + existingWindow.addTabbedWindow(window, ordered: .above) + } } } From 772eba89ae1957e94ac041bae7ad7929fffbeb42 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 24 Mar 2026 11:40:04 +0700 Subject: [PATCH 04/10] fix: add diagnostic logging for tab grouping --- TablePro/AppDelegate+WindowConfig.swift | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/TablePro/AppDelegate+WindowConfig.swift b/TablePro/AppDelegate+WindowConfig.swift index 29198fda..ef2c9748 100644 --- a/TablePro/AppDelegate+WindowConfig.swift +++ b/TablePro/AppDelegate+WindowConfig.swift @@ -252,11 +252,13 @@ extension AppDelegate { // Explicitly attach to existing tab group — automatic tabbing // doesn't work when tabbingIdentifier is set after window creation. - if let existingWindow = NSApp.windows.first(where: { - $0 !== window && isMainWindow($0) && $0.isVisible - && $0.tabbingIdentifier == resolvedIdentifier - }) { + let mainWindows = NSApp.windows.filter { $0 !== window && isMainWindow($0) && $0.isVisible } + windowLogger.info( + "New main window: resolved=\(resolvedIdentifier), pendingId=\(pendingId?.uuidString ?? "nil"), existing=\(mainWindows.map { $0.tabbingIdentifier })" + ) + if let existingWindow = mainWindows.first(where: { $0.tabbingIdentifier == resolvedIdentifier }) { existingWindow.addTabbedWindow(window, ordered: .above) + window.makeKeyAndOrderFront(nil) } } } From 4273a4d9aecfc97aecfd67d5205d40c118de5dd2 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 24 Mar 2026 11:42:59 +0700 Subject: [PATCH 05/10] fix: add NSLog diagnostic for windowDidBecomeKey --- TablePro/AppDelegate+WindowConfig.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/TablePro/AppDelegate+WindowConfig.swift b/TablePro/AppDelegate+WindowConfig.swift index ef2c9748..58807fec 100644 --- a/TablePro/AppDelegate+WindowConfig.swift +++ b/TablePro/AppDelegate+WindowConfig.swift @@ -214,6 +214,12 @@ extension AppDelegate { @objc func windowDidBecomeKey(_ notification: Notification) { guard let window = notification.object as? NSWindow else { return } let windowId = ObjectIdentifier(window) + NSLog("[WindowConfig] didBecomeKey: id=%@, isMain=%d, configured=%d, title=%@, tabbing=%@", + window.identifier?.rawValue ?? "nil", + isMainWindow(window) ? 1 : 0, + configuredWindows.contains(windowId) ? 1 : 0, + window.title, + window.tabbingIdentifier) if isWelcomeWindow(window) && isHandlingFileOpen { window.close() From d73146d07b7ac5f5578913c9d48535ff2162c0bd Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 24 Mar 2026 11:46:49 +0700 Subject: [PATCH 06/10] fix: match SwiftUI window identifiers with hasPrefix instead of exact match SwiftUI appends -AppWindow-N to the WindowGroup id, so window.identifier is "main-AppWindow-1" not "main". The exact match from PR #441 broke isMainWindow, preventing tab grouping and window lifecycle handling. --- TablePro/AppDelegate+WindowConfig.swift | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/TablePro/AppDelegate+WindowConfig.swift b/TablePro/AppDelegate+WindowConfig.swift index 58807fec..74af2530 100644 --- a/TablePro/AppDelegate+WindowConfig.swift +++ b/TablePro/AppDelegate+WindowConfig.swift @@ -107,7 +107,8 @@ extension AppDelegate { } func isMainWindow(_ window: NSWindow) -> Bool { - window.identifier?.rawValue == WindowId.main + guard let rawValue = window.identifier?.rawValue else { return false } + return rawValue == WindowId.main || rawValue.hasPrefix("\(WindowId.main)-") } func isWelcomeWindow(_ window: NSWindow) -> Bool { @@ -214,12 +215,6 @@ extension AppDelegate { @objc func windowDidBecomeKey(_ notification: Notification) { guard let window = notification.object as? NSWindow else { return } let windowId = ObjectIdentifier(window) - NSLog("[WindowConfig] didBecomeKey: id=%@, isMain=%d, configured=%d, title=%@, tabbing=%@", - window.identifier?.rawValue ?? "nil", - isMainWindow(window) ? 1 : 0, - configuredWindows.contains(windowId) ? 1 : 0, - window.title, - window.tabbingIdentifier) if isWelcomeWindow(window) && isHandlingFileOpen { window.close() @@ -258,11 +253,10 @@ extension AppDelegate { // Explicitly attach to existing tab group — automatic tabbing // doesn't work when tabbingIdentifier is set after window creation. - let mainWindows = NSApp.windows.filter { $0 !== window && isMainWindow($0) && $0.isVisible } - windowLogger.info( - "New main window: resolved=\(resolvedIdentifier), pendingId=\(pendingId?.uuidString ?? "nil"), existing=\(mainWindows.map { $0.tabbingIdentifier })" - ) - if let existingWindow = mainWindows.first(where: { $0.tabbingIdentifier == resolvedIdentifier }) { + if let existingWindow = NSApp.windows.first(where: { + $0 !== window && isMainWindow($0) && $0.isVisible + && $0.tabbingIdentifier == resolvedIdentifier + }) { existingWindow.addTabbedWindow(window, ordered: .above) window.makeKeyAndOrderFront(nil) } From b217f76a6a881cf8d768b0baaa1947438c0b1b05 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 24 Mar 2026 11:52:18 +0700 Subject: [PATCH 07/10] fix: use prefix matching for window identifiers across all call sites SwiftUI appends -AppWindow-N to WindowGroup IDs. Fixed remaining spots that used .contains("main") or exact match: - closeRestoredMainWindows: use isMainWindow helper - closeWindows(withId:): prefix match instead of contains - ContentView notification filter: prefix match instead of contains --- TablePro/AppDelegate+WindowConfig.swift | 4 ++-- TablePro/ContentView.swift | 3 ++- .../Extensions/NSApplication+WindowManagement.swift | 12 +++++++----- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/TablePro/AppDelegate+WindowConfig.swift b/TablePro/AppDelegate+WindowConfig.swift index 74af2530..82a53314 100644 --- a/TablePro/AppDelegate+WindowConfig.swift +++ b/TablePro/AppDelegate+WindowConfig.swift @@ -329,8 +329,8 @@ extension AppDelegate { } func closeRestoredMainWindows() { - DispatchQueue.main.async { - for window in NSApp.windows where window.identifier?.rawValue.contains("main") == true { + DispatchQueue.main.async { [weak self] in + for window in NSApp.windows where self?.isMainWindow(window) == true { window.close() } } diff --git a/TablePro/ContentView.swift b/TablePro/ContentView.swift index 9ee562e6..1bbf967f 100644 --- a/TablePro/ContentView.swift +++ b/TablePro/ContentView.swift @@ -114,7 +114,8 @@ struct ContentView: View { // Match by checking if the window is registered for our connectionId // in WindowLifecycleMonitor (subtitle may not be set yet on first appear). guard let notificationWindow = notification.object as? NSWindow, - notificationWindow.identifier?.rawValue.contains("main") == true, + let windowId = notificationWindow.identifier?.rawValue, + windowId == "main" || windowId.hasPrefix("main-"), let connectionId = payload?.connectionId else { return } diff --git a/TablePro/Extensions/NSApplication+WindowManagement.swift b/TablePro/Extensions/NSApplication+WindowManagement.swift index 96c16f0b..fd201eb4 100644 --- a/TablePro/Extensions/NSApplication+WindowManagement.swift +++ b/TablePro/Extensions/NSApplication+WindowManagement.swift @@ -10,12 +10,14 @@ import AppKit extension NSApplication { - /// Close all windows whose identifier contains the given ID. - /// Legacy workaround from when the minimum was macOS 13. Now that macOS 14+ is the minimum, - /// callers could use SwiftUI's `dismissWindow(id:)` instead. + /// Close all windows whose identifier matches the given ID (exact or SwiftUI-suffixed). + /// SwiftUI appends "-AppWindow-N" to WindowGroup IDs, so we match by prefix. func closeWindows(withId id: String) { - for window in windows where window.identifier?.rawValue.contains(id) == true { - window.close() + for window in windows { + guard let rawValue = window.identifier?.rawValue else { continue } + if rawValue == id || rawValue.hasPrefix("\(id)-") { + window.close() + } } } } From 4eec577a7f8fceb6d05767c5b35f84d03620dba2 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 24 Mar 2026 12:06:44 +0700 Subject: [PATCH 08/10] fix: address code review feedback - Use queue: .main instead of assumeIsolated in DataGridView teardown observer - Apply hasPrefix matching to isWelcomeWindow and isConnectionFormWindow - Gate RowBuffer/QueryTabManager deinit logging behind #if DEBUG - Add CHANGELOG entries for memory, tab grouping, and sidebar fixes --- CHANGELOG.md | 3 +++ TablePro/AppDelegate+WindowConfig.swift | 6 ++++-- TablePro/Models/Query/QueryTab.swift | 4 ++++ TablePro/Views/Results/DataGridView.swift | 7 ++----- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e6a0a34..b3d2b3f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Health monitor now detects stuck queries beyond the configured timeout - SSH tunnel closure errors now logged instead of silently discarded - Schema/database restore errors during reconnect now logged +- Memory not reclaimed after closing many tabs (RowBuffer, NSTableView cell views, editor state now released during teardown) +- New tabs opening as separate windows instead of attaching to connection tab group +- Sidebar table clicks not opening table tabs (isKeyWindow not set after WindowAccessor migration) ## [0.23.1] - 2026-03-24 diff --git a/TablePro/AppDelegate+WindowConfig.swift b/TablePro/AppDelegate+WindowConfig.swift index 82a53314..f443d2bf 100644 --- a/TablePro/AppDelegate+WindowConfig.swift +++ b/TablePro/AppDelegate+WindowConfig.swift @@ -112,11 +112,13 @@ extension AppDelegate { } func isWelcomeWindow(_ window: NSWindow) -> Bool { - window.identifier?.rawValue == WindowId.welcome + guard let rawValue = window.identifier?.rawValue else { return false } + return rawValue == WindowId.welcome || rawValue.hasPrefix("\(WindowId.welcome)-") } private func isConnectionFormWindow(_ window: NSWindow) -> Bool { - window.identifier?.rawValue == WindowId.connectionForm + guard let rawValue = window.identifier?.rawValue else { return false } + return rawValue == WindowId.connectionForm || rawValue.hasPrefix("\(WindowId.connectionForm)-") } // MARK: - Welcome Window diff --git a/TablePro/Models/Query/QueryTab.swift b/TablePro/Models/Query/QueryTab.swift index 89fe602c..a58b187b 100644 --- a/TablePro/Models/Query/QueryTab.swift +++ b/TablePro/Models/Query/QueryTab.swift @@ -272,8 +272,10 @@ final class RowBuffer { } deinit { + #if DEBUG Logger(subsystem: "com.TablePro", category: "RowBuffer") .debug("RowBuffer deallocated — columns: \(self.columns.count), evicted: \(self.isEvicted)") + #endif } } @@ -684,7 +686,9 @@ final class QueryTabManager { } deinit { + #if DEBUG Logger(subsystem: "com.TablePro", category: "QueryTabManager") .debug("QueryTabManager deallocated") + #endif } } diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index 1497fd28..790ce392 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -848,12 +848,9 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData teardownObserver = NotificationCenter.default.addObserver( forName: MainContentCoordinator.teardownNotification, object: connectionId, - queue: nil + queue: .main ) { [weak self] _ in - guard let self else { return } - MainActor.assumeIsolated { - self.releaseData() - } + self?.releaseData() } } From b2bbfb910cbf39310b05e25b73b25f89cdd049b0 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 24 Mar 2026 12:20:07 +0700 Subject: [PATCH 09/10] fix: guard buffer writes against nil to avoid mutating shared emptyBuffer --- TablePro/Models/Query/RowProvider.swift | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/TablePro/Models/Query/RowProvider.swift b/TablePro/Models/Query/RowProvider.swift index 221e5054..2a81e1a1 100644 --- a/TablePro/Models/Query/RowProvider.swift +++ b/TablePro/Models/Query/RowProvider.swift @@ -163,7 +163,8 @@ final class InMemoryRowProvider: RowProvider { guard rowIndex < totalRowCount else { return } let sourceIndex = resolveSourceIndex(rowIndex) if let bufferIdx = sourceIndex.bufferIndex { - safeBuffer.rows[bufferIdx][columnIndex] = value + guard let buffer = rowBuffer else { return } + buffer.rows[bufferIdx][columnIndex] = value displayCache.removeValue(forKey: bufferIdx) } else if let appendedIdx = sourceIndex.appendedIndex { appendedRows[appendedIdx][columnIndex] = value @@ -231,7 +232,8 @@ final class InMemoryRowProvider: RowProvider { /// Update rows by replacing the buffer contents and clearing appended rows func updateRows(_ newRows: [[String?]]) { - safeBuffer.rows = newRows + guard let buffer = rowBuffer else { return } + buffer.rows = newRows appendedRows.removeAll() sortIndices = nil displayCache.removeAll() @@ -254,9 +256,10 @@ final class InMemoryRowProvider: RowProvider { guard appendedIdx < appendedRows.count else { return } appendedRows.remove(at: appendedIdx) } else { + guard let buffer = rowBuffer else { return } if let sorted = sortIndices { let bufferIdx = sorted[index] - safeBuffer.rows.remove(at: bufferIdx) + buffer.rows.remove(at: bufferIdx) var newIndices = sorted newIndices.remove(at: index) for i in newIndices.indices where newIndices[i] > bufferIdx { @@ -264,7 +267,7 @@ final class InMemoryRowProvider: RowProvider { } sortIndices = newIndices } else { - safeBuffer.rows.remove(at: index) + buffer.rows.remove(at: index) } } displayCache.removeAll() From 02310b066603f1b9455ce221276b1aae3d8afcfc Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 24 Mar 2026 12:21:17 +0700 Subject: [PATCH 10/10] docs: simplify changelog entries --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3d2b3f7..dc483082 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,9 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Health monitor now detects stuck queries beyond the configured timeout - SSH tunnel closure errors now logged instead of silently discarded - Schema/database restore errors during reconnect now logged -- Memory not reclaimed after closing many tabs (RowBuffer, NSTableView cell views, editor state now released during teardown) -- New tabs opening as separate windows instead of attaching to connection tab group -- Sidebar table clicks not opening table tabs (isKeyWindow not set after WindowAccessor migration) +- Memory not released after closing tabs +- New tabs opening as separate windows instead of joining the connection tab group +- Clicking tables in sidebar not opening table tabs ## [0.23.1] - 2026-03-24