fix(deps): stop libsql closing every connection twice (#367, #308)

[oscaruiz]

Summary

• Fixes Windows CI: libsql-backed test binaries crash with STATUS_ACCESS_VIOLATION at process exit, after every test passes (follow-up to #308) #367.
• Also explains Flaky: Windows CI test binaries intermittently crash with STATUS_ACCESS_VIOLATION (0xc0000005) #308, which was closed on 2026-07-30 on the stack-exhaustion theory. The crash came back, which is why Windows CI: libsql-backed test binaries crash with STATUS_ACCESS_VIOLATION at process exit, after every test passes (follow-up to #308) #367 exists; the suite Flaky: Windows CI test binaries intermittently crash with STATUS_ACCESS_VIOLATION (0xc0000005) #308 was filed against (resolution_test) reproduces here and is clean after this change. Whether to reopen it or leave it closed as superseded is your call.
• libsql closes every local connection twice. The second sqlite3_close_v2 reads an already-freed sqlite3 object, which is a use-after-free on every platform and an intermittent STATUS_ACCESS_VIOLATION on Windows.
• Fixed by vendoring the published libsql 0.9.30 crate with a one-line change and routing to it through [patch.crates-io], because the bug is still present upstream.

Motivation

libsql::local::Connection::disconnect is not idempotent:

LibsqlConnection::drop  -> self.conn.disconnect()   // local/impls.rs:110  first close
Connection::drop        -> self.disconnect()        // local/connection.rs:40 second close

The Arc::get_mut(drop_ref) guard only proves unique ownership, which is true on both passes, and raw is left dangling after the first close.

This was counted, not inferred. Instrumenting the driver with a print around the FFI call:

suite	sqlite3_open_v2	sqlite3_close_v2	SQLITE_OK	SQLITE_MISUSE
db_query_test	76	152	76	76
discover_test + gain_test (GlobalDb path)	5	10	5	5
with this fix	81	81	81	0

Exactly two closes per handle, the second always failing its state check because the handle is gone.

On glibc and macOS the freed page stays mapped, so the second close returns SQLITE_MISUSE harmlessly — that part is measured. That the same read faults on Windows once the block has been recycled or decommitted is the explanation for the platform asymmetry, not a second measurement. It also explains the timing #367 reports: heap corruption from a double free detonates later, at process exit, after every test has already printed ok.

Neither #181's RUST_MIN_STACK nor #337's /STACK:8388608 could have addressed this. Both target stack exhaustion; this is a heap lifetime bug. That is also why the crash survived #308 being closed: the fix that closed it was aimed at a different mechanism.

Changes

• vendor/libsql/ — a copy of the published libsql 0.9.30 crate. Of its 44 source files, only src/local/connection.rs differs from the published crate, by two lines plus the comment explaining why. Upstream's MIT licence is included. Examples, tests, benches, test assets, lockfile and contributor notes are dropped; [lints] and [workspace] tables are added to its manifest.
• Cargo.toml — [patch.crates-io] pointing libsql at the vendored copy, with the rationale and the condition for removing it.
• Cargo.lock — libsql resolves through the path patch instead of the registry.
• tests/vendored_libsql_test.rs — four guards, so the patch cannot be lost silently: the patch entry and the vendored directory must exist together; a source checkout must carry the patch; the vendored version must match the requested one; and the fix must both be present and close the handle before nulling it, since the inverse order would pass a naive string check while closing nothing.
• scripts/win-stress.ps1 — the batch runner used for the measurements below. It counts access violations separately from other non-zero exits, and takes a -TempDir so a long batch does not fill the system drive.
• .gitignore — ignore the separate Windows target directory that script uses.

Test plan

• cargo test --workspace passes, with one pre-existing exception noted below
• cargo clippy --workspace --all-targets has no new warnings
• Tested manually (see below)

Run against this branch's head: 67 suites green, clippy clean, cargo fmt --all -- --check clean.

The single failure is mcp_handler_test::run_affected_tests_dispatches_directly_changed_test_files. It fails identically on master without this branch — 190 passed, 1 failed in both — so it is pre-existing and unrelated. It passes when run on its own, so it looks like a timing flake under suite load rather than a real regression.

The meaningful verification is on Windows, since the crash cannot be reproduced on Linux or macOS. 500 iterations per suite, per build:

suite	unpatched	patched
db_query_test (named in #367)	29/500 (5.8%)	0/500
accounting_test (named in #367)	10/500 (2.0%)	0/500
resolution_test (named in #308)	13/500 (2.6%)	0/500
db_test	—	0/500
graph_test	—	0/100
extraction_test (opens no database, control)	—	0/500

Every failure is exit code 0xC0000005, and every one lands after the last test has printed ok. Had the rate been unchanged, 500 clean runs would carry probability around 10⁻¹³.

Two controls, both on the unpatched build, retire the standing theories:

• --test-threads=1 does not hide the crash but roughly triples it (47/300), so parallel test execution is not what drives it.
• RUST_MIN_STACK at 32 MB leaves the rate unchanged (12/300), so no amount of stack is relevant.

All 111 test binaries in the workspace were also built for Windows and run twice with this fix and #367's companion drop-order fix applied together: 0 access violations, 0 other failures across 222 runs. That sweep bounds the two changes jointly, not this one alone.

These numbers come from the x86_64-pc-windows-gnu toolchain; this machine has no Visual Studio Build Tools. The crash reproduces there anyway, which shows it does not depend on the MSVC CRT or on #337's link flag, but equivalence with the MSVC toolchain CI uses has not been demonstrated directly.

Note on scope

cargo package strips [patch.crates-io] and leaves vendor/libsql out of the published archive — verified against the generated .crate. So this covers CI and anyone building from a checkout, while binaries produced by cargo install tokensave keep the upstream double free until libsql itself ships the fix.

The bug is reported upstream as libsql#2251, with the equivalent fix waiting in libsql#2261 since July. Both 0.9.30 and 0.10.0-pre.4 still carry it. The vendored copy and the [patch] entry should be deleted together the moment an upstream release lands; tests/vendored_libsql_test.rs is written so that removing all three at once is the only clean way to do it.

Checklist

• CHANGELOG.md updated under [Unreleased]
• No secrets, credentials, or .env files included
• Breaking changes documented: none. No change to tokensave's own source, public API, or behaviour on any platform; the only runtime difference is that a connection is closed once instead of twice.
• Third-party licence: upstream libsql's MIT notice is included at vendor/libsql/LICENSE.md.

[aovestdipaperino]

Thanks for this — the double-close was a nasty use-after-free and the 500-run batching made the fix airtight. Merged.