Skip to content

fix(plugins): synchronize the driver adapter's shared column type cache#1864

Merged
datlechin merged 1 commit into
mainfrom
fix/plugin-driver-adapter-data-race
Jul 13, 2026
Merged

fix(plugins): synchronize the driver adapter's shared column type cache#1864
datlechin merged 1 commit into
mainfrom
fix/plugin-driver-adapter-data-race

Conversation

@datlechin

Copy link
Copy Markdown
Member

Problem

A user reported random crashes with several tables open: sometimes on switching table, sometimes on reload, sometimes on returning to the app, once about 30 seconds after launch. No crash log was available.

PluginDriverAdapter is the app-side bridge every query goes through, and there is exactly one instance per connection: driver(for:) returns activeSessions[connectionId]?.driver, so every tab, every window and the health monitor share it.

It held a bare, unsynchronized dictionary:

private var columnTypeCache: [String: ColumnType] = [:]

private func mapColumnType(rawTypeName: String) -> ColumnType {
    if let cached = columnTypeCache[rawTypeName] { return cached }
    let result = classifier.classify(rawTypeName: rawTypeName)
    columnTypeCache[rawTypeName] = result   // unsynchronized write
    return result
}

mapQueryResult calls it once per column, on execute, executeParameterized and executeUserQuery. Those are reached from QueryExecutor.fetchQueryData, which is nonisolated static and runs on the cooperative pool. The plugins' serial queues protect the C calls, but the mapping runs after the await resumes, back on the concurrent pool. Two queries on one connection can be mapping columns at the same time.

Concurrent Dictionary mutation corrupts the backing store. That is undefined behavior, and it shows up exactly as reported: crashes with no reproducible step, because the damage is done long before the process notices.

The reason this went unnoticed is the protocol. PluginDatabaseDriver (plugin side) is already AnyObject, Sendable, but the app-side DatabaseDriver was only AnyObject. Nothing ever required a type shared across executors to be safe to share, and strict concurrency is not enabled, so the compiler never asked.

Fix

  • status and columnTypeCache move into a State struct behind OSAllocatedUnfairLock, the primitive already used elsewhere in the app. Type mapping takes the lock once per result rather than once per column.
  • DatabaseDriver gains the Sendable constraint it was missing, so the compiler enforces this from now on.

PluginDriverAdapter is now genuinely Sendable, not @unchecked: every stored property is Sendable (DatabaseConnection is all value types, ColumnType's payloads are String?/[String]?, ColumnTypeClassifier is a pure struct with a static let lookup).

The only other conformer is a test mock, which gains @unchecked Sendable.

Tests

PluginDriverAdapterConcurrencyTests drives the interleaving that corrupts the cache today:

  • 64 concurrent executeUserQuery calls through one adapter, asserting every caller gets correctly mapped column types
  • mixed concurrent execute / executeUserQuery sharing the cache
  • concurrent status reads while connect and disconnect run

These pass with the lock. Without it they race, and under Thread Sanitizer they report on columnTypeCache.

Notes

This is a verified data race that is capable of producing the reported crash, but with no .ips from the reporter I cannot prove it is the one they hit. Three further defects found during the investigation are queued as separate PRs:

  • the health monitor's ping-skip guard is dead code, because the data grid's query path never calls trackOperation, so queriesInFlight is always empty
  • MySQL's cancelCurrentQuery() dereferences the raw MYSQL* on the caller's thread, off the connection's serial queue, while disconnect() nils that pointer without holding stateLock
  • the data grid branch has no .id(tab.id), so SwiftUI reuses one TableViewCoordinator across different table tabs

@datlechin datlechin merged commit 98ff008 into main Jul 13, 2026
1 of 2 checks passed
@datlechin datlechin deleted the fix/plugin-driver-adapter-data-race branch July 13, 2026 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant