Skip to content

Commit cb9a272

Browse files
authored
fix: release memory when closing tabs (#434)
* 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 * 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. * 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. * fix: add diagnostic logging for tab grouping * fix: add NSLog diagnostic for windowDidBecomeKey * 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. * 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 * 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 * fix: guard buffer writes against nil to avoid mutating shared emptyBuffer * docs: simplify changelog entries
1 parent c50b52a commit cb9a272

14 files changed

Lines changed: 209 additions & 21 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1616
- Health monitor now detects stuck queries beyond the configured timeout
1717
- SSH tunnel closure errors now logged instead of silently discarded
1818
- Schema/database restore errors during reconnect now logged
19+
- Memory not released after closing tabs
20+
- New tabs opening as separate windows instead of joining the connection tab group
21+
- Clicking tables in sidebar not opening table tabs
1922

2023
## [0.23.1] - 2026-03-24
2124

LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/Controller/TextViewController.swift

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,31 @@ public class TextViewController: NSViewController {
290290
self.gutterView.setNeedsDisplay(self.gutterView.frame)
291291
}
292292

293+
/// Release heavy resources (tree-sitter, highlighter, text storage) early,
294+
/// without waiting for deinit. Call when the editor is no longer visible but
295+
/// SwiftUI may keep the controller alive in @State.
296+
public func releaseHeavyState() {
297+
if let highlighter {
298+
textView?.removeStorageDelegate(highlighter)
299+
}
300+
highlighter = nil
301+
treeSitterClient = nil
302+
highlightProviders.removeAll()
303+
// Don't call textCoordinators.destroy() here — the caller (coordinator.destroy())
304+
// is already a coordinator, so calling back into destroy() causes infinite recursion.
305+
textCoordinators.removeAll()
306+
cancellables.forEach { $0.cancel() }
307+
cancellables.removeAll()
308+
if let localEventMonitor {
309+
NSEvent.removeMonitor(localEventMonitor)
310+
}
311+
localEventMonitor = nil
312+
textView?.setText("")
313+
}
314+
293315
deinit {
294316
if let highlighter {
295-
textView.removeStorageDelegate(highlighter)
317+
textView?.removeStorageDelegate(highlighter)
296318
}
297319
highlighter = nil
298320
highlightProviders.removeAll()

TablePro/AppDelegate+WindowConfig.swift

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,18 @@ extension AppDelegate {
107107
}
108108

109109
func isMainWindow(_ window: NSWindow) -> Bool {
110-
window.identifier?.rawValue == WindowId.main
110+
guard let rawValue = window.identifier?.rawValue else { return false }
111+
return rawValue == WindowId.main || rawValue.hasPrefix("\(WindowId.main)-")
111112
}
112113

113114
func isWelcomeWindow(_ window: NSWindow) -> Bool {
114-
window.identifier?.rawValue == WindowId.welcome
115+
guard let rawValue = window.identifier?.rawValue else { return false }
116+
return rawValue == WindowId.welcome || rawValue.hasPrefix("\(WindowId.welcome)-")
115117
}
116118

117119
private func isConnectionFormWindow(_ window: NSWindow) -> Bool {
118-
window.identifier?.rawValue == WindowId.connectionForm
120+
guard let rawValue = window.identifier?.rawValue else { return false }
121+
return rawValue == WindowId.connectionForm || rawValue.hasPrefix("\(WindowId.connectionForm)-")
119122
}
120123

121124
// MARK: - Welcome Window
@@ -239,15 +242,26 @@ extension AppDelegate {
239242
let existingIdentifier = NSApp.windows
240243
.first { $0 !== window && isMainWindow($0) && $0.isVisible }?
241244
.tabbingIdentifier
242-
window.tabbingIdentifier = TabbingIdentifierResolver.resolve(
245+
let resolvedIdentifier = TabbingIdentifierResolver.resolve(
243246
pendingConnectionId: pendingId,
244247
existingIdentifier: existingIdentifier
245248
)
249+
window.tabbingIdentifier = resolvedIdentifier
246250
configuredWindows.insert(windowId)
247251

248252
if !NSWindow.allowsAutomaticWindowTabbing {
249253
NSWindow.allowsAutomaticWindowTabbing = true
250254
}
255+
256+
// Explicitly attach to existing tab group — automatic tabbing
257+
// doesn't work when tabbingIdentifier is set after window creation.
258+
if let existingWindow = NSApp.windows.first(where: {
259+
$0 !== window && isMainWindow($0) && $0.isVisible
260+
&& $0.tabbingIdentifier == resolvedIdentifier
261+
}) {
262+
existingWindow.addTabbedWindow(window, ordered: .above)
263+
window.makeKeyAndOrderFront(nil)
264+
}
251265
}
252266
}
253267

@@ -317,8 +331,8 @@ extension AppDelegate {
317331
}
318332

319333
func closeRestoredMainWindows() {
320-
DispatchQueue.main.async {
321-
for window in NSApp.windows where window.identifier?.rawValue.contains("main") == true {
334+
DispatchQueue.main.async { [weak self] in
335+
for window in NSApp.windows where self?.isMainWindow(window) == true {
322336
window.close()
323337
}
324338
}

TablePro/ContentView.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ struct ContentView: View {
114114
// Match by checking if the window is registered for our connectionId
115115
// in WindowLifecycleMonitor (subtitle may not be set yet on first appear).
116116
guard let notificationWindow = notification.object as? NSWindow,
117-
notificationWindow.identifier?.rawValue.contains("main") == true,
117+
let windowId = notificationWindow.identifier?.rawValue,
118+
windowId == "main" || windowId.hasPrefix("main-"),
118119
let connectionId = payload?.connectionId
119120
else { return }
120121

TablePro/Extensions/NSApplication+WindowManagement.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010
import AppKit
1111

1212
extension NSApplication {
13-
/// Close all windows whose identifier contains the given ID.
14-
/// Legacy workaround from when the minimum was macOS 13. Now that macOS 14+ is the minimum,
15-
/// callers could use SwiftUI's `dismissWindow(id:)` instead.
13+
/// Close all windows whose identifier matches the given ID (exact or SwiftUI-suffixed).
14+
/// SwiftUI appends "-AppWindow-N" to WindowGroup IDs, so we match by prefix.
1615
func closeWindows(withId id: String) {
17-
for window in windows where window.identifier?.rawValue.contains(id) == true {
18-
window.close()
16+
for window in windows {
17+
guard let rawValue = window.identifier?.rawValue else { continue }
18+
if rawValue == id || rawValue.hasPrefix("\(id)-") {
19+
window.close()
20+
}
1921
}
2022
}
2123
}

TablePro/Models/Query/QueryTab.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import Foundation
99
import Observation
10+
import os
1011
import TableProPluginKit
1112

1213
/// Type of tab
@@ -269,6 +270,13 @@ final class RowBuffer {
269270
self.rows = newRows
270271
isEvicted = false
271272
}
273+
274+
deinit {
275+
#if DEBUG
276+
Logger(subsystem: "com.TablePro", category: "RowBuffer")
277+
.debug("RowBuffer deallocated — columns: \(self.columns.count), evicted: \(self.isEvicted)")
278+
#endif
279+
}
272280
}
273281

274282
/// Represents a single tab (query or table)
@@ -676,4 +684,11 @@ final class QueryTabManager {
676684
tabs[index] = tab
677685
}
678686
}
687+
688+
deinit {
689+
#if DEBUG
690+
Logger(subsystem: "com.TablePro", category: "QueryTabManager")
691+
.debug("QueryTabManager deallocated")
692+
#endif
693+
}
679694
}

TablePro/Models/Query/RowProvider.swift

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,12 @@ final class TableRowData {
6666
/// Direct-access methods `value(atRow:column:)` and `rowValues(at:)` avoid
6767
/// heap allocations by reading straight from the source `[String?]` array.
6868
final class InMemoryRowProvider: RowProvider {
69-
private let rowBuffer: RowBuffer
69+
private weak var rowBuffer: RowBuffer?
70+
/// Strong reference only when the provider created its own buffer (convenience init).
71+
/// External buffers are owned by QueryTab, so we hold them weakly.
72+
private var ownedBuffer: RowBuffer?
73+
private static let emptyBuffer = RowBuffer()
74+
private var safeBuffer: RowBuffer { rowBuffer ?? Self.emptyBuffer }
7075
private var sortIndices: [Int]?
7176
private var appendedRows: [[String?]] = []
7277
private(set) var columns: [String]
@@ -86,7 +91,7 @@ final class InMemoryRowProvider: RowProvider {
8691

8792
/// Number of rows coming from the buffer (respecting sort indices count when present)
8893
private var bufferRowCount: Int {
89-
sortIndices?.count ?? rowBuffer.rows.count
94+
sortIndices?.count ?? safeBuffer.rows.count
9095
}
9196

9297
init(
@@ -130,6 +135,7 @@ final class InMemoryRowProvider: RowProvider {
130135
columnEnumValues: columnEnumValues,
131136
columnNullable: columnNullable
132137
)
138+
ownedBuffer = buffer
133139
}
134140

135141
func fetchRows(offset: Int, limit: Int) -> [TableRowData] {
@@ -157,7 +163,8 @@ final class InMemoryRowProvider: RowProvider {
157163
guard rowIndex < totalRowCount else { return }
158164
let sourceIndex = resolveSourceIndex(rowIndex)
159165
if let bufferIdx = sourceIndex.bufferIndex {
160-
rowBuffer.rows[bufferIdx][columnIndex] = value
166+
guard let buffer = rowBuffer else { return }
167+
buffer.rows[bufferIdx][columnIndex] = value
161168
displayCache.removeValue(forKey: bufferIdx)
162169
} else if let appendedIdx = sourceIndex.appendedIndex {
163170
appendedRows[appendedIdx][columnIndex] = value
@@ -215,9 +222,18 @@ final class InMemoryRowProvider: RowProvider {
215222
displayCache.removeAll()
216223
}
217224

225+
/// Release cached data to free memory when this provider is no longer active.
226+
func releaseData() {
227+
displayCache.removeAll()
228+
appendedRows.removeAll()
229+
sortIndices = nil
230+
ownedBuffer = nil
231+
}
232+
218233
/// Update rows by replacing the buffer contents and clearing appended rows
219234
func updateRows(_ newRows: [[String?]]) {
220-
rowBuffer.rows = newRows
235+
guard let buffer = rowBuffer else { return }
236+
buffer.rows = newRows
221237
appendedRows.removeAll()
222238
sortIndices = nil
223239
displayCache.removeAll()
@@ -240,17 +256,18 @@ final class InMemoryRowProvider: RowProvider {
240256
guard appendedIdx < appendedRows.count else { return }
241257
appendedRows.remove(at: appendedIdx)
242258
} else {
259+
guard let buffer = rowBuffer else { return }
243260
if let sorted = sortIndices {
244261
let bufferIdx = sorted[index]
245-
rowBuffer.rows.remove(at: bufferIdx)
262+
buffer.rows.remove(at: bufferIdx)
246263
var newIndices = sorted
247264
newIndices.remove(at: index)
248265
for i in newIndices.indices where newIndices[i] > bufferIdx {
249266
newIndices[i] -= 1
250267
}
251268
sortIndices = newIndices
252269
} else {
253-
rowBuffer.rows.remove(at: index)
270+
buffer.rows.remove(at: index)
254271
}
255272
}
256273
displayCache.removeAll()
@@ -297,9 +314,9 @@ final class InMemoryRowProvider: RowProvider {
297314
return appendedRows[displayIndex - bCount]
298315
}
299316
if let sorted = sortIndices {
300-
return rowBuffer.rows[sorted[displayIndex]]
317+
return safeBuffer.rows[sorted[displayIndex]]
301318
}
302-
return rowBuffer.rows[displayIndex]
319+
return safeBuffer.rows[displayIndex]
303320
}
304321
}
305322

TablePro/Resources/Localizable.xcstrings

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27181,6 +27181,9 @@
2718127181
}
2718227182
}
2718327183
}
27184+
},
27185+
"SSH Connection Test Failed" : {
27186+
2718427187
},
2718527188
"SSH connection timed out" : {
2718627189
"localizations" : {

TablePro/Views/Editor/SQLEditorCoordinator.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,22 @@ final class SQLEditorCoordinator: TextViewCoordinator {
146146
inlineSuggestionManager?.uninstall()
147147
inlineSuggestionManager = nil
148148

149+
// Release closure captures to break potential retain cycles
150+
onCloseTab = nil
151+
onExecuteQuery = nil
152+
onAIExplain = nil
153+
onAIOptimize = nil
154+
onSaveAsFavorite = nil
155+
schemaProvider = nil
156+
contextMenu = nil
157+
vimEngine = nil
158+
vimCursorManager = nil
159+
160+
// Release editor controller heavy state
161+
controller?.releaseHeavyState()
162+
149163
EditorEventRouter.shared.unregister(self)
164+
Self.logger.debug("SQLEditorCoordinator destroyed")
150165
cleanupMonitors()
151166
}
152167

TablePro/Views/Editor/SQLEditorView.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ struct SQLEditorView: View {
126126
.onDisappear {
127127
teardownFavoritesObserver()
128128
coordinator.destroy()
129+
completionAdapter = nil
129130
}
130131
.onChange(of: coordinator.vimMode) { _, newMode in
131132
vimMode = newMode

0 commit comments

Comments
 (0)