Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Cancel now stops a connection attempt right away instead of letting it run on in the background. A cancelled connection is also dropped from the last session, so restarting no longer reconnects to a host you gave up on, and it can no longer interrupt a later successful connection. (#1358)
- Fixed a crash when clicking a column header on a table whose columns have comments. Sorting such a table quit the app immediately. (#1869)

## [0.57.0] - 2026-07-14
Expand Down
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ Missing a case produces a wrong "{Language} Query" title on the first frame.

**Selection indices are display positions**: `GridSelectionState.indices` come from `NSTableView.selectedRowIndexes` and are display-row positions, not indices into `TableRows.rows`. They match array indices only when `displayIDs` (`valueFilteredIDs ?? sortedIDs`) is nil; a per-column value filter makes them diverge. Resolve any selected index through `DisplayRowMapping` (or `TableViewCoordinator.displayRow(at:)` / `tableRowsIndex(forDisplayRow:)`) before reading or mutating a row; never index `TableRows.rows` with a display position. The row details inspector shipped this bug (#1837).

**Cancelling a connect does not stop the driver**: `Task.cancel()` is cooperative, so it cannot interrupt a driver blocked in a C call. A cancelled attempt keeps running and completes late. Two rules follow. First, a driver that blocks on connect must expose its own abort path and poll it (the PostgreSQL driver uses `PQconnectStart`/`PQconnectPoll` with an app-owned deadline and a cancel flag flipped from `withTaskCancellationHandler`; a blocking `PQconnectdb` cannot be cancelled at all). Second, never assume the losing attempt is gone: every attempt validates its `ConnectionAttemptRegistry` generation before adopting a driver into `activeSessions` or tearing session state down, so a late attempt discards its own driver instead of clobbering the winner. Cancelling also drops the connection from `LastOpenConnections.json` (via `MainContentCoordinator.syncRecoveryList()`, activated windows only) so "Reopen Last Session" never replays a connect the user cancelled. This area shipped the same bug four times (#1185, #1358, #1369).

### Main Coordinator Pattern

`MainContentCoordinator` is the central coordinator, split across 7+ extension files in `Views/Main/Extensions/` (e.g., `+Alerts`, `+Filtering`, `+Pagination`, `+RowOperations`). When adding coordinator functionality, add a new extension file rather than growing the main file.
Expand Down
204 changes: 146 additions & 58 deletions Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
message: String(localized: "Not connected to database"), sqlState: nil, detail: nil)
static let connectionFailed = LibPQPluginError(
message: String(localized: "Failed to establish connection"), sqlState: nil, detail: nil)
static let connectionTimedOut = LibPQPluginError(
message: String(localized: "Timed out while connecting to the server"), sqlState: nil, detail: nil)
}

// MARK: - Query Result
Expand Down Expand Up @@ -84,6 +86,9 @@
// MARK: - Connection Class

final class LibPQPluginConnection: @unchecked Sendable {
private static let connectTimeoutMicroseconds: Int64 = 10_000_000
private static let pollSliceMicroseconds: Int64 = 100_000

private var conn: OpaquePointer?
private let queue = DispatchQueue(label: "com.TablePro.libpq.plugin", qos: .userInitiated)

Expand All @@ -101,6 +106,7 @@
private var _cachedServerVersion: String?
private var _cachedServerVersionNumber: Int32 = 0
private var _isCancelled: Bool = false
private var _isConnectCancelled: Bool = false
private var _postgisOidMap: [UInt32: String] = [:]

var isConnected: Bool {
Expand Down Expand Up @@ -154,86 +160,168 @@
// MARK: - Connection Management

func connect() async throws {
try await pluginDispatchAsyncCancellable(on: queue) { [self] in
func escapeConnParam(_ value: String) -> String {
value.replacingOccurrences(of: "\\", with: "\\\\")
.replacingOccurrences(of: "'", with: "\\'")
stateLock.lock()

Check warning on line 163 in Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift

View workflow job for this annotation

GitHub Actions / macOS App Tests

instance method 'lock' is unavailable from asynchronous contexts; Use async-safe scoped locking instead; this is an error in the Swift 6 language mode
_isConnectCancelled = false
stateLock.unlock()

Check warning on line 165 in Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift

View workflow job for this annotation

GitHub Actions / macOS App Tests

instance method 'unlock' is unavailable from asynchronous contexts; Use async-safe scoped locking instead; this is an error in the Swift 6 language mode

try await withTaskCancellationHandler {
try await pluginDispatchAsyncCancellable(
on: queue,
cancellationCheck: { [weak self] in self?.isConnectCancelled ?? true }
) { [self] in
try performConnect()
}
} onCancel: {
cancelConnect()
}
}

var connStr = "host='\(escapeConnParam(host))' port='\(port)' dbname='\(escapeConnParam(database))' connect_timeout='10'"
func cancelConnect() {
stateLock.lock()
_isConnectCancelled = true
stateLock.unlock()
}

if !user.isEmpty {
connStr += " user='\(escapeConnParam(user))'"
}
private var isConnectCancelled: Bool {
stateLock.lock()
defer { stateLock.unlock() }
return _isConnectCancelled
}

if let password = password, !password.isEmpty {
connStr += " password='\(escapeConnParam(password))'"
}
private func performConnect() throws {
guard let connection = buildConnectionString().withCString({ PQconnectStart($0) }) else {
throw LibPQPluginError.connectionFailed
}

connStr += " sslmode='\(LibPQSSLMapping.sslmode(for: sslConfig.mode))'"
var adopted = false
defer {
if !adopted { PQfinish(connection) }
}

if sslConfig.verifiesCertificate, !sslConfig.caCertificatePath.isEmpty {
connStr += " sslrootcert='\(escapeConnParam(sslConfig.caCertificatePath))'"
}
if !sslConfig.clientCertificatePath.isEmpty {
connStr += " sslcert='\(escapeConnParam(sslConfig.clientCertificatePath))'"
}
if !sslConfig.clientKeyPath.isEmpty {
connStr += " sslkey='\(escapeConnParam(sslConfig.clientKeyPath))'"
}
guard PQstatus(connection) != CONNECTION_BAD else {
throw connectionError(from: connection)
}

if let options, !options.isEmpty {
connStr += " options='\(escapeConnParam(options))'"
}
try pollUntilConnected(connection)
configureEstablishedConnection(connection)

let connection = connStr.withCString { cStr in
PQconnectdb(cStr)
}
stateLock.lock()
conn = connection
_isConnected = true
stateLock.unlock()
adopted = true
}

guard let connection = connection else {
throw LibPQPluginError.connectionFailed
private func pollUntilConnected(_ connection: OpaquePointer) throws {
let deadline = PQgetCurrentTimeUSec() + Self.connectTimeoutMicroseconds

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve libpq multi-host failover

When the PostgreSQL host field contains a comma-separated failover list and an earlier host blackholes, this single 10s deadline aborts the whole PQconnectPoll sequence before libpq can advance to the next host. The previous connect_timeout=10 behavior allowed libpq to time out each host separately, so a healthy later host could still be reached; with this global deadline those HA connections fail as soon as the first host consumes the budget.

Useful? React with 👍 / 👎.

var status = PGRES_POLLING_WRITING

while true {
try checkConnectCancellation()

switch status {
case PGRES_POLLING_OK:
return
case PGRES_POLLING_FAILED:
throw connectionError(from: connection)
case PGRES_POLLING_READING, PGRES_POLLING_WRITING:
let socket = PQsocket(connection)
guard socket >= 0 else { throw connectionError(from: connection) }

let now = PQgetCurrentTimeUSec()
guard now < deadline else { throw LibPQPluginError.connectionTimedOut }

let ready = PQsocketPoll(
socket,
status == PGRES_POLLING_READING ? 1 : 0,
status == PGRES_POLLING_WRITING ? 1 : 0,
min(deadline, now + Self.pollSliceMicroseconds)
)
guard ready >= 0 else { throw LibPQPluginError.connectionFailed }
guard ready > 0 else { continue }

status = PQconnectPoll(connection)
default:
status = PQconnectPoll(connection)
}
}
}

if PQstatus(connection) != CONNECTION_OK {
let error = self.getError(from: connection)
PQfinish(connection)
if let sslError = LibPQSSLClassifier.classifySSLError(error.message) {
throw sslError
}
throw error
}
private func checkConnectCancellation() throws {
guard isConnectCancelled else { return }
throw CancellationError()
}

"SET client_encoding TO 'UTF8'".withCString { cStr in
let result = PQexec(connection, cStr)
PQclear(result)
}
private func connectionError(from connection: OpaquePointer) -> Error {
let error = getError(from: connection)
if let sslError = LibPQSSLClassifier.classifySSLError(error.message) {
return sslError
}
return error
}

let version = PQserverVersion(connection)
if version > 0 {
self._cachedServerVersionNumber = version
let major = version / 10_000
if major >= 10 {
let minor = version % 10_000
self._cachedServerVersion = "\(major).\(minor)"
} else {
let minor = (version / 100) % 100
let revision = version % 100
self._cachedServerVersion = "\(major).\(minor).\(revision)"
}
}
private func configureEstablishedConnection(_ connection: OpaquePointer) {
"SET client_encoding TO 'UTF8'".withCString { cStr in
let result = PQexec(connection, cStr)
PQclear(result)
}

let version = PQserverVersion(connection)
guard version > 0 else { return }

_cachedServerVersionNumber = version
let major = version / 10_000
if major >= 10 {
let minor = version % 10_000
_cachedServerVersion = "\(major).\(minor)"
} else {
let minor = (version / 100) % 100
let revision = version % 100
_cachedServerVersion = "\(major).\(minor).\(revision)"
}
}

private func buildConnectionString() -> String {
func escapeConnParam(_ value: String) -> String {
value.replacingOccurrences(of: "\\", with: "\\\\")
.replacingOccurrences(of: "'", with: "\\'")
}

var connStr = "host='\(escapeConnParam(host))' port='\(port)' dbname='\(escapeConnParam(database))'"

if !user.isEmpty {
connStr += " user='\(escapeConnParam(user))'"
}

if let password, !password.isEmpty {
connStr += " password='\(escapeConnParam(password))'"
}

connStr += " sslmode='\(LibPQSSLMapping.sslmode(for: sslConfig.mode))'"

self.stateLock.lock()
self.conn = connection
self._isConnected = true
self.stateLock.unlock()
if sslConfig.verifiesCertificate, !sslConfig.caCertificatePath.isEmpty {
connStr += " sslrootcert='\(escapeConnParam(sslConfig.caCertificatePath))'"
}
if !sslConfig.clientCertificatePath.isEmpty {
connStr += " sslcert='\(escapeConnParam(sslConfig.clientCertificatePath))'"
}
if !sslConfig.clientKeyPath.isEmpty {
connStr += " sslkey='\(escapeConnParam(sslConfig.clientKeyPath))'"
}

if let options, !options.isEmpty {
connStr += " options='\(escapeConnParam(options))'"
}

return connStr
}

func disconnect() {
isShuttingDown = true

stateLock.lock()
_isConnected = false
_isConnectCancelled = true
let handle = conn
conn = nil
stateLock.unlock()
Expand Down
6 changes: 1 addition & 5 deletions TablePro/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
}

private func persistOpenConnectionsForRecovery() {
var seen = Set<UUID>()
let connectionIds = MainContentCoordinator.activeCoordinators.values
.map(\.connectionId)
.filter { seen.insert($0).inserted }
LastOpenConnectionsStorage.shared.save(connectionIds: connectionIds)
LastOpenConnectionsStorage.shared.save(connectionIds: SessionRecoveryTracker.connectionIds())
}

@objc func handleSystemDidWake(_ notification: Notification) {
Expand Down
30 changes: 30 additions & 0 deletions TablePro/Core/Database/ConnectionAttemptRegistry.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//
// ConnectionAttemptRegistry.swift
// TablePro
//

import Foundation

struct ConnectionAttemptRegistry {
private var generations: [UUID: Int] = [:]
private var lastGeneration: Int = 0

mutating func begin(for connectionId: UUID) -> Int {
lastGeneration += 1
generations[connectionId] = lastGeneration
return lastGeneration
}

func isCurrent(_ generation: Int, for connectionId: UUID) -> Bool {
generations[connectionId] == generation
}

mutating func invalidate(for connectionId: UUID) {
generations.removeValue(forKey: connectionId)
}

mutating func finish(_ generation: Int, for connectionId: UUID) {
guard generations[connectionId] == generation else { return }
generations.removeValue(forKey: connectionId)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ extension DatabaseManager {
}

func cancelEnsureConnected(_ connectionId: UUID) async {
connectionAttempts.invalidate(for: connectionId)
await ensureConnectedDedup.cancel(key: connectionId)
if let session = activeSessions[connectionId], session.driver == nil {
if let tunnelManager = activeTunnelManager(for: session.connection) {
Expand Down
Loading
Loading