fix: do not block statement cleanup on a busy connection

[warmwaffles]

statement_type_destructor took the connection mutex and called sqlite3_finalize. A connection in the busy handler holds that mutex, and SQLite's db mutex, until busy_timeout expires. Cleanup then stalled the process that held the write lock, so the waiting BEGIN IMMEDIATE failed with "database is locked".

The destructor now finalizes only when the connection mutex is free. Otherwise it queues the statement. The next locked call, or connection close, finalizes it.

Grok 4.7 assisted with creating this fix. Most difficult part was making a test that demonstrated the failure repeatedly.

Fixes: #360

[warmwaffles]

Linter is failing, I'll fix that in a bit. Just want to validate that this resolves the issue.

[mm503]

Excited about this getting in!

[andreasronge]

Tested fix/360 with the repro from #360 on macOS, busy_timeout 3000, with the collected statement prepared on the waiting connection. Each entry is the holder’s GC + SELECT time in ms and the waiter’s result:

build	busy timeout via	runs
fix/360	set_busy_timeout/2	1/ok 0/ok 3/ok 0/ok 0/ok 1/ok 0/ok 0/ok
fix/360	PRAGMA busy_timeout	0/ok 1/ok 0/ok 0/ok 0/ok 4/ok 17/ok 5/ok

Before the fix, 16 of 25 runs on 0.34.0–0.40.0 stalled for about 2.9 s and the waiter failed with database is locked. No stall was observed in 16 branch runs.

The resource-lifetime path appears sound: the queue node is allocated during prepare; destruction either obtains the connection mutex or enqueues the statement; each statement retains the connection resource until that is complete; and connection destruction drains before sqlite3_close_v2. Explicit release nulls the SQLite statement, so the later resource destructor only frees its unused queue slot.

Two remaining edge cases:

1. release/2 still uses a blocking acquisition of the statement’s owning connection mutex before sqlite3_finalize. The connection argument is only validated; it does not determine which mutex is acquired. A manually retained query prepared on one pooled connection and closed while using another can therefore reproduce the same wait on a dirty I/O scheduler. This does not look like a common Ecto query path, but giving explicit release the same defer-on-contention behavior would cover it.
2. A queued statement is normally finalized on the owner’s next locked operation, when a later statement destructor successfully obtains the mutex, or during connection destruction. Exqlite.Connection.ping/1 does not enter the NIF, so an otherwise idle pooled connection may retain queued statements indefinitely. This is only delayed reclamation for a reset/completed statement. A low-level statement dropped after returning a row can retain its WAL read transaction until finalization and prevent a checkpoint from advancing past that reader.

I have not measured the second case.

This investigation (reproduction runs, source review, and this write-up) was done with Claude Opus 5.5 and GPT-5.6 Sol (via Codex).

[warmwaffles]

Awesome news. I'll take a look into the two remaining points and see what's possible.

[warmwaffles]

@andreasronge I believe this should address all of your points above.

[andreasronge]

Thanks, and sorry, just being a meat proxy here for my AIs :)

This covers both points. On 329a94f the #360 repro gives no stall in 16 runs (holder 0–20 ms, waiter ok, both busy-handler variants), and test/exqlite/sqlite3_test.exs passes locally. The enqueue/unlock handshake looks free of lost wakeups, and nulling statement->statement before dropping the retained reference keeps an inline destructor on the free-only path.

Two minor things from 9d38306, neither blocking:

1. The unlock drain can replace an error message. Several NIFs build the error tuple after connection_release_lock, which now finalizes queued statements. Finalizing a statement that stopped mid-result (not reset) overwrites the connection's error, so a failed execute could return {:error, "not an error"}. Statements stepped to done or error are reset by exqlite and don't trigger it, so this needs a low-level statement dropped after returning rows, queued at the moment another call fails. Building the error term before unlocking avoids it. It's worth doing because callers classify busy errors by message text.
2. release/2's contended path reads statement->statement under finalize_mutex while other paths write it under conn->mutex. It needs two concurrent releases of the same statement and I don't see a double free, but checking only slot there would avoid the data race.

Reviewed with Claude Opus 5.5 and GPT-6.1 Sol (via Codex).

[jounimakela]

Tested 329a94f locally: 180s run, 10 connections, 20 short-lived writer processes, busy_timeout 250ms, about 200 writes/s.

I didn't see any stalls. For comparison, 0.40.0 had 307 stalls in one run.

I did attempt to get some idea of memory use with :erlang.memory(:system). Starts at 31.6 MB, rises to 33.3 MB and stays there, drops back to 31.5 MB after the connections close. So no sign of a leak.

I also pushed the fix to a test device (aarch64) and can report back after it has had some time to run.

[jounimakela]

The test device has now ran ~24 hours with zero "Database busy" errors. The setup has pool size of 10, Oban stager interval back at default (1 s), busy_timeout at 5 s.

Thanks @warmwaffles! Looking forward to 0.42.0!

[warmwaffles]

Cool, let me button up the last bit of things here and I'll get this in.

[warmwaffles]

This has been released under v0.42.0