Skip to content

feat(plugins): hand back a DuckDB file lock or MySQL connection when idle - #2677

Merged
datlechin merged 2 commits into
mainfrom
feat/duckdb-release-file-lock
Sep 8, 2026
Merged

feat(plugins): hand back a DuckDB file lock or MySQL connection when idle#2677
datlechin merged 2 commits into
mainfrom
feat/duckdb-release-file-lock

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #2518.

Root cause

duckdb_open_ext takes a whole-file POSIX fcntl write lock that lives exactly as long as the duckdb_database handle, and DuckDBConnectionActor held that handle from connect until disconnect(). So an open TablePro tab owned the file for the whole session and nothing else on the machine could open it.

Measured against the shipped Libs/libduckdb_arm64.a (v1.5.2) with C probes:

  • fcntl(F_GETLK) on the file while a handle is open reports F_WRLCK, l_start=0, l_len=0, l_pid = the holder. A read-only handle reports F_RDLCK.
  • duckdb_close is the only release. It costs 0.3-1.8ms, checkpoints, deletes the .wal, and another process wins the lock on its first attempt ~4.5ms later. A reopen costs 3-14ms.
  • Two processes can both hold access_mode=READ_ONLY (three concurrent readers verified). A read-write holder blocks even a read-only open; a read-only holder blocks a writer.

What ships

Idle release, opt-in per connection. Release the File Lock After (minutes, 0 keeps it) in the DuckDB connection's Options. The handle closes once the connection has been idle that long and reopens on the next query, replaying the driver's own setup: the two autoload SETs and the USE position.

Release File Lock, on the Database menu beside Disconnect and on the connections strip's context menu. The session, its tabs and its schema cache all stay.

Open the File Read-Only, using access_mode=READ_ONLY, so several processes can read one file at once.

A lock-conflict message that names the holder. DuckDB's own text already carries the holder's executable path, PID and user; TablePro was surfacing it raw. DuckDBLockConflict parses it and the message reads lockprobe is using t.duckdb. Only one app can open a DuckDB file for writing., plus the read-only route when DuckDB says it would work.

The release is refused when the session has something to lose

Measured: a close and reopen destroys temporary tables and views, every changed SET, every ATTACHed catalog and the USE position, and rolls an open transaction back raising nothing. So "invisible" is only truthful when there is nothing to lose, and the precondition is measured rather than guessed:

  • duckdb_databases() WHERE internal = false above one means an ATTACH.
  • duckdb_tables() and duckdb_views() WHERE temporary AND internal = false. The internal = false is load-bearing: a connection that has run nothing already reports 46 temporary views, so without it the lock would never be released at all.
  • A duckdb_settings() snapshot diff against connect time, excluding the settings the driver moves itself. duckdb_settings() reports no default, so the baseline is the only way to see a user change. Measured across a table create, a scan and a metadata read: zero of 152 settings drift on their own, so this does not block the feature from ever firing.
  • The driver's own transaction tracking.

The check and the duckdb_close happen in one non-suspending actor call, so a statement cannot land between them and be destroyed.

MySQL

The same generic hook, with the same refusal contract, and off by default. Release the Server Connection After in Options.

Measured against MySQL 8.4.11 and MariaDB 12.3.3, the detection has to work differently. An ordinary MySQL 8 user is refused on every table that would report its own session: error 1142 on performance_schema.user_variables_by_thread, prepared_statements_instances, metadata_locks and events_transactions_current, and error 1227 on information_schema.INNODB_TRX and INNODB_TEMP_TABLE_INFO. GET_LOCK has no enumeration for anybody. So MySQLSessionFootprint reads the statements instead, which costs no privilege, no round trip, and behaves identically on MariaDB: a temporary table, a user variable, a prepared statement, an advisory lock, LOCK TABLES, a changed session setting, an open transaction, or a CALL whose body is opaque all keep the connection.

The prize is smaller than DuckDB's and the cost is higher: an idle connection costs MariaDB ~186KB and one slot of 151, nothing is blocked on it, and reconnecting is 2ms on loopback but 800-1900ms across the internet. That is why it is off by default and why the docs say so.

The health ping would have killed both

The health monitor pings every 30 seconds through execute, so a ping that counted as activity would hold the idle clock at zero and the timer would never once fire; and a ping that reconnected a deliberately released connection would undo the release 30 seconds after it happened. ping() on both drivers now neither counts as activity nor re-acquires.

PluginKit 23

Two defaulted PluginDatabaseDriver requirements, releasableResourceCommandTitle and releaseIdleResource, plus the PluginResourceRelease they answer with, and SQLTransactionTracking shared by both drivers. All additive, nothing removed.

The bump is still required: per #2597 a rebuilt plugin hard-references the new method descriptors, so a shipped v22 app would accept it and then fail to load it. minimumCompatiblePluginKitVersion stays at 19, so every already-published plugin keeps loading.

Release checklist: ./scripts/release-all-plugins.sh 23 must run before or with the next app release, or users on the new app hit noCompatibleBinary on every registry plugin.

Also fixed, found while investigating

  • connectRemote leaked a whole DuckDB instance on every failed attempt. It opened the handle and then ran three throwing statements with no cleanup, while connectLocal had already been fixed for exactly this; DatabaseManager disconnects only a cancelled attempt, the actor has no deinit, and close() is reachable only from disconnect(). Any unreachable host triggers it. Measured over 20 failed attempts: RSS 3.2MB to 64.2MB, threads 1 to 221. It blocks the primary fix because open() has no guard against overwriting a live handle, which is what the reopen path calls.
  • MySQL replayed a statement outside the transaction it belonged to. executeWithReconnect reconnects and replays on errors 2006/2013/2055, guarded only by mysqlStatementIsSafeToReplay, with no check for an open transaction. It now refuses to replay across one.
  • The query timeout was lost after any MySQL reconnect. reconnect() now replays it.
  • A stale duckdb_interrupt pointer. It was captured once at connect, which was correct only while the handle outlived nothing. The live pointer now moves with the handle, cleared before duckdb_disconnect and set after duckdb_connect; a cancel landing in the gap reads nil.
  • A comment claiming libduckdb documents duckdb_interrupt as thread-safe. The vendored header says nothing of the kind: zero hits for thread-safe, concurrently or another thread across its 6,279 lines.

Verification

Step Result
generate PASS
build (app + 18 bundled plugins) PASS
build DuckDBDriver / build MySQLDriver PASS
test (9 suites, 70 cases) PASS
lint over TablePro, both plugins, PluginKit, TableProTests 0 findings on the changed files
docs writing style + against-source PASS
abi vs 3496162c diff is purely additive: 0 removals, and the additions are the two requirements, PluginResourceRelease and SQLTransactionTracking

Two local steps could not give a clean answer, neither caused by this change:

  • plugins (the AllPlugins aggregate) fails on the vendored oracle-nio's @TaskLocal macro, unknown attribute 'usableFromInlinenonisolated'. That is the only error in the log and it is pre-existing on this toolchain. DuckDBDriver and MySQLDriver were built as individual targets instead, both PASS, and CI runs the aggregate on its own toolchain.
  • lint reports two legacy_swiftui_aspect_ratio errors in ImportFromAppSourcePicker.swift and SupportView.swift. This branch does not touch either file.

The tests caught a real defect in the first run, and it is worth naming because it is the dangerous direction: splitting a batch on ; cuts a string literal, so SELECT 'a; COMMIT ' produced a fragment whose first word is COMMIT and read as a transaction ending. That clears the flag protecting a real transaction, and the release that follows rolls it back. Closing now requires the whole statement to be a transaction statement; opening still needs only the first word, because over-reporting an open only keeps the resource longer.

Screenshots

Not included, and the reason is specific rather than an omission.

The DuckDB plugin is registry-only, so a local Debug build does not carry it. Choosing DuckDB in the connection sheet offers the plugin download first, and the registry currently publishes PluginKit 22 binaries, which do not have this change in them. Database ▸ Release File Lock validates against a live driver, so it cannot be shown enabled without that plugin, and the DuckDB Options pane cannot be shown with the driver actually behind it.

The two new fields do render from the curated snapshot without the plugin, so the connection form is reachable; driving it far enough to frame the Options section was not worth taking over the machine while three other build sessions were running on it. Every visible change is a ConnectionField and two menu items, all covered by DuckDBConnectionFieldsTests, which pins the field ids, order, section, types and defaults.

https://claude.ai/code/session_01YXH6cGRcEouTMwNdkv495g

@mintlify

mintlify Bot commented Sep 8, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 8, 2026, 4:21 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

@datlechin

Copy link
Copy Markdown
Member Author

Codex review round (145144a49)

A Codex review raised 13 findings. Twelve were real and are fixed; one is rejected with evidence.

The one that mattered most: applyQueryTimeout ran its SET SESSION through the tracked path, and DatabaseManager applies the timeout on every connect. The MySQL footprint was therefore dirty before the user ran anything, so the MySQL release could never have fired. It now goes in as driver setup.

Root-caused rather than patched: two findings (a ; inside a string literal reading as a COMMIT, and a leading comment hiding a statement's keyword) were both the same defect, a raw split(separator: ";"). New SQLStatementSplitting in PluginKit tracks quotes, backticks, dollar quotes and both comment forms, and strips leading comments. Both the transaction tracker and the MySQL footprint use it.

Verified by probe, not taken on trust: the claim that DuckDB temp macros, PREPARE and SET VARIABLE escape the gate. Measured on the shipped v1.5.2, all three leave both temporary counts at zero while appearing in duckdb_functions(), duckdb_prepared_statements() and duckdb_variables(). The gate now sums all five catalogs.

Also fixed: a duckdb_interrupt use-after-free (the box no longer hands the pointer out, interrupt() holds the lock across the call); reconnecting a session the user explicitly disconnected; a release racing in-flight work (added an active-operation count); an uncancellable MySQL stream bridge I had introduced; a swallowed USE replay failure on reopen; SELECT @x := 1; a DuckDB transaction left untracked when a batch failed part-way; and a refusal alert that said "file" to someone who released a server connection.

Rejected: "keep PluginKit at 22, additive needs no bump." The quote from CLAUDE.md is accurate, but that rule is wrong, and this PR corrects it. It describes only the old-plugin-in-new-host direction. The reverse was measured in #2597 and is already documented in PluginManager.swift: a rebuilt plugin hard-references the new method descriptor and the default-implementation symbol, so a v22 app accepts it (validateBundleVersions only rejects a version above its own) and then fails Bundle.loadAndReturnError. Versions 21 and 22 were each such a bump. 23 stays.

Re-verified after the fixes: app build, DuckDBDriver, MySQLDriver, 75 test cases across 9 suites, lint, and both docs checks. All pass.

@datlechin
datlechin merged commit 5ada6a7 into main Sep 8, 2026
6 checks passed
@datlechin
datlechin deleted the feat/duckdb-release-file-lock branch September 8, 2026 16:39
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.

Release the DuckDB file lock when a connection goes idle

1 participant