diff --git a/CHANGELOG.md b/CHANGELOG.md index 595497639..6b7b6730d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed a random crash when more than one table was open on the same connection. Queries running at the same time shared an unguarded type cache, which could corrupt memory and take the app down on a table switch, a reload, or when coming back to the app. - Column comments now show in the data grid header as soon as the table opens, instead of only after the query was run again. The header also grows to fit the comment line, so it is no longer cut off until a column is resized. (#1861) - **Size to Fit** and **Size All Columns to Fit** no longer stretch a column with long text far past the window. A fitted column now stops at half the visible grid, and every column has a maximum width, so a column left oversized before is brought back in range the next time you open the table. - The username is now optional. Leaving it empty, or importing a connection URL that has no user in it, no longer fills in `root`. An empty username lets the database use its own default, the same as `psql` and `mysql` do: your Mac login name on MySQL/MariaDB and PostgreSQL, and `default` on ClickHouse. The `~/.pgpass` lookup now matches on that same user. diff --git a/TablePro/Core/Database/DatabaseDriver.swift b/TablePro/Core/Database/DatabaseDriver.swift index 7d37446d2..b627b2bbe 100644 --- a/TablePro/Core/Database/DatabaseDriver.swift +++ b/TablePro/Core/Database/DatabaseDriver.swift @@ -10,7 +10,7 @@ import OSLog import TableProPluginKit /// Protocol defining database driver operations -protocol DatabaseDriver: AnyObject { +protocol DatabaseDriver: AnyObject, Sendable { // MARK: - Properties /// The connection configuration diff --git a/TablePro/Core/Plugins/PluginDriverAdapter.swift b/TablePro/Core/Plugins/PluginDriverAdapter.swift index 28b44a469..67f745a2f 100644 --- a/TablePro/Core/Plugins/PluginDriverAdapter.swift +++ b/TablePro/Core/Plugins/PluginDriverAdapter.swift @@ -8,11 +8,19 @@ import os import TableProPluginKit final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable { + private struct State { + var status: ConnectionStatus = .disconnected + var columnTypeCache: [String: ColumnType] = [:] + } + let connection: DatabaseConnection - private(set) var status: ConnectionStatus = .disconnected private let pluginDriver: any PluginDatabaseDriver - private var columnTypeCache: [String: ColumnType] = [:] private let classifier = ColumnTypeClassifier() + private let state = OSAllocatedUnfairLock(initialState: State()) + + var status: ConnectionStatus { + state.withLock { $0.status } + } var serverVersion: String? { pluginDriver.serverVersion } var parameterStyle: ParameterStyle { pluginDriver.parameterStyle } @@ -101,19 +109,19 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable { // MARK: - Connection Management func connect() async throws { - status = .connecting + state.withLock { $0.status = .connecting } do { try await pluginDriver.connect() - status = .connected + state.withLock { $0.status = .connected } } catch { - status = .error(error.localizedDescription) + state.withLock { $0.status = .error(error.localizedDescription) } throw error } } func disconnect() { pluginDriver.disconnect() - status = .disconnected + state.withLock { $0.status = .disconnected } } func ping() async throws { @@ -660,7 +668,7 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable { // MARK: - Result Mapping private func mapQueryResult(_ pluginResult: PluginQueryResult) -> QueryResult { - let columnTypes = pluginResult.columnTypeNames.map { mapColumnType(rawTypeName: $0) } + let columnTypes = mapColumnTypes(rawTypeNames: pluginResult.columnTypeNames) var result = QueryResult( columns: pluginResult.columns, columnTypes: columnTypes, @@ -677,11 +685,15 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable { return result } - private func mapColumnType(rawTypeName: String) -> ColumnType { - if let cached = columnTypeCache[rawTypeName] { return cached } - let result = classifier.classify(rawTypeName: rawTypeName) - columnTypeCache[rawTypeName] = result - return result + private func mapColumnTypes(rawTypeNames: [String]) -> [ColumnType] { + state.withLock { state in + rawTypeNames.map { rawTypeName in + if let cached = state.columnTypeCache[rawTypeName] { return cached } + let mapped = classifier.classify(rawTypeName: rawTypeName) + state.columnTypeCache[rawTypeName] = mapped + return mapped + } + } } } diff --git a/TableProTests/Core/Database/PostgreSQLDriverTests.swift b/TableProTests/Core/Database/PostgreSQLDriverTests.swift index 9d3aa846e..9b90ff919 100644 --- a/TableProTests/Core/Database/PostgreSQLDriverTests.swift +++ b/TableProTests/Core/Database/PostgreSQLDriverTests.swift @@ -163,7 +163,7 @@ struct PostgreSQLDDLAssembly { // MARK: - DDL Loading Flow Mock -private final class MockPostgreSQLDriver: DatabaseDriver { +private final class MockPostgreSQLDriver: DatabaseDriver, @unchecked Sendable { let connection: DatabaseConnection var status: ConnectionStatus = .connected var serverVersion: String? = "16.0.0" diff --git a/TableProTests/Core/Plugins/PluginDriverAdapterConcurrencyTests.swift b/TableProTests/Core/Plugins/PluginDriverAdapterConcurrencyTests.swift new file mode 100644 index 000000000..abf20307d --- /dev/null +++ b/TableProTests/Core/Plugins/PluginDriverAdapterConcurrencyTests.swift @@ -0,0 +1,149 @@ +// +// PluginDriverAdapterConcurrencyTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +@Suite("PluginDriverAdapter shares one instance per connection across tabs and windows") +struct PluginDriverAdapterConcurrencyTests { + private static let columnTypeNames = [ + "VARCHAR(255)", "INT", "BIGINT", "DECIMAL(10,2)", "DATE", "TIMESTAMP", + "DATETIME", "BOOLEAN", "BLOB", "JSON", "TEXT", "SMALLINT", + "DOUBLE", "FLOAT", "CHAR(10)", "TINYINT" + ] + + private func makeAdapter() -> PluginDriverAdapter { + PluginDriverAdapter( + connection: TestFixtures.makeConnection(type: .mysql), + pluginDriver: ConcurrentStubPluginDriver(columnTypeNames: Self.columnTypeNames) + ) + } + + private func expectedTypes() -> [ColumnType] { + let classifier = ColumnTypeClassifier() + return Self.columnTypeNames.map { classifier.classify(rawTypeName: $0) } + } + + @Test("Concurrent executeUserQuery calls on one adapter all map column types correctly") + func concurrentUserQueriesMapColumnTypes() async throws { + let adapter = makeAdapter() + let expected = expectedTypes() + + let results = await withTaskGroup(of: [ColumnType].self) { group in + for _ in 0..<64 { + group.addTask { + let result = try? await adapter.executeUserQuery( + query: "SELECT * FROM t", + rowCap: nil, + parameters: nil + ) + return result?.columnTypes ?? [] + } + } + return await group.reduce(into: [[ColumnType]]()) { $0.append($1) } + } + + #expect(results.count == 64) + for columnTypes in results { + #expect(columnTypes == expected) + } + } + + @Test("Concurrent execute and executeUserQuery calls share the type cache without losing entries") + func concurrentMixedQueriesShareTypeCache() async throws { + let adapter = makeAdapter() + let expected = expectedTypes() + + await withTaskGroup(of: Void.self) { group in + for index in 0..<64 { + group.addTask { + if index.isMultiple(of: 2) { + _ = try? await adapter.execute(query: "SELECT 1") + } else { + _ = try? await adapter.executeUserQuery( + query: "SELECT * FROM t", + rowCap: nil, + parameters: nil + ) + } + } + } + } + + let result = try await adapter.executeUserQuery(query: "SELECT * FROM t", rowCap: nil, parameters: nil) + #expect(result.columnTypes == expected) + } + + @Test("Reading status while connect and disconnect run concurrently stays a valid state") + func concurrentStatusReadsStayValid() async throws { + let adapter = makeAdapter() + let valid: [ConnectionStatus] = [.disconnected, .connecting, .connected] + + await withTaskGroup(of: Void.self) { group in + for index in 0..<32 { + group.addTask { + if index.isMultiple(of: 2) { + try? await adapter.connect() + } else { + adapter.disconnect() + } + } + group.addTask { + #expect(valid.contains(adapter.status)) + } + } + } + } +} + +private final class ConcurrentStubPluginDriver: PluginDatabaseDriver, @unchecked Sendable { + private let columnTypeNames: [String] + + init(columnTypeNames: [String]) { + self.columnTypeNames = columnTypeNames + } + + private func makeResult() -> PluginQueryResult { + PluginQueryResult( + columns: columnTypeNames.indices.map { "col_\($0)" }, + columnTypeNames: columnTypeNames, + rows: [], + rowsAffected: 0, + executionTime: 0.001 + ) + } + + func connect() async throws {} + func disconnect() {} + + func execute(query: String) async throws -> PluginQueryResult { + await Task.yield() + return makeResult() + } + + func executeParameterized(query: String, parameters: [PluginCellValue]) async throws -> PluginQueryResult { + await Task.yield() + return makeResult() + } + + func executeUserQuery( + query: String, + rowCap: Int?, + parameters: [PluginCellValue]? + ) async throws -> PluginQueryResult { + await Task.yield() + return makeResult() + } + + func fetchTables(schema: String?) async throws -> [PluginTableInfo] { [] } + func fetchColumns(table: String, schema: String?) async throws -> [PluginColumnInfo] { [] } + func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] { [] } + func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { [] } + func fetchTableDDL(table: String, schema: String?) async throws -> String { "" } + func fetchViewDefinition(view: String, schema: String?) async throws -> String { "" } + func fetchDatabases() async throws -> [String] { [] } +}