From 0070ff3331cd6d09425b812e1cd3ebe32e1d4206 Mon Sep 17 00:00:00 2001
From: Pekka Enberg <penberg@iki.fi>
Date: Wed, 16 Sep 2026 11:12:35 +0300
Subject: [PATCH] libsql: close a local connection's sqlite3 handle exactly
 once

LibsqlConnection::drop called local::Connection::disconnect(), and then
the inner local::Connection's own Drop called disconnect() again on the
same raw handle, so every connection close ran sqlite3_close_v2() twice.
The second call reads the freed sqlite3 struct. It usually returns
SQLITE_MISUSE harmlessly, but when another thread has reused the freed
block first, sqlite walks garbage and crashes. This surfaced in libsql-js
as an intermittent access violation at worker-thread exit on Windows,
where the finalizers close many connections in a burst.

Make disconnect() idempotent by forgetting the handle after closing it,
and drop the redundant Drop impl on LibsqlConnection, since the inner
connection already closes on drop.

Add a regression test that counts sqlite3_close_v2() calls through a
cfg(test) thread-local and checks a drop closes the handle exactly once,
both for a plain drop and when a statement is the last owner. It fails
with 2 closes on the old code.
---
 libsql/src/local/connection.rs | 49 ++++++++++++++++++++++++++++++++++
 libsql/src/local/impls.rs      |  6 -----
 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/libsql/src/local/connection.rs b/libsql/src/local/connection.rs
index 7012651699..5986d2af59 100644
--- a/libsql/src/local/connection.rs
+++ b/libsql/src/local/connection.rs
@@ -22,6 +22,14 @@ struct Container {
     cb: Box<UpdateHook>,
 }
 
+#[cfg(test)]
+thread_local! {
+    /// Number of `sqlite3_close_v2` calls made by [`Connection::disconnect`]
+    /// on this thread, so tests can check that a drop closes a handle exactly
+    /// once.
+    static CLOSE_COUNT: std::cell::Cell<usize> = std::cell::Cell::new(0);
+}
+
 /// A connection to a libSQL database.
 #[derive(Clone)]
 pub struct Connection {
@@ -105,9 +113,20 @@ impl Connection {
     }
 
     /// Disconnect from the database.
+    ///
+    /// Closes the underlying handle once the last clone of this connection
+    /// is gone. Calling this more than once is harmless: the handle is
+    /// forgotten after it is closed, so a later call (including the one from
+    /// `Drop`) never touches the freed `sqlite3`.
     pub fn disconnect(&mut self) {
+        if self.raw.is_null() {
+            return;
+        }
         if Arc::get_mut(&mut self.drop_ref).is_some() {
             unsafe { libsql_sys::ffi::sqlite3_close_v2(self.raw) };
+            #[cfg(test)]
+            CLOSE_COUNT.with(|count| count.set(count.get() + 1));
+            self.raw = std::ptr::null_mut();
         }
     }
 
@@ -812,12 +831,42 @@ extern "C" fn update_hook_cb(
 
 #[cfg(test)]
 mod tests {
+    use super::CLOSE_COUNT;
     use crate::{
         local::{Connection, Database},
         params::Params,
         OpenFlags,
     };
 
+    /// Dropping the last owner of a connection must close the `sqlite3`
+    /// handle exactly once. `LibsqlConnection` used to close it from its own
+    /// `Drop` and then again from the inner `Connection`'s `Drop`, so the
+    /// second `sqlite3_close_v2` ran on freed memory.
+    #[tokio::test]
+    async fn drop_closes_the_handle_exactly_once() {
+        let closes = || CLOSE_COUNT.with(|count| count.get());
+        let db = crate::Database::open(":memory:").unwrap();
+
+        // Nothing else refers to the connection.
+        let conn = db.connect().unwrap();
+        let before = closes();
+        drop(conn);
+        assert_eq!(closes() - before, 1);
+
+        // A statement keeps the connection alive and is its last owner.
+        let conn = db.connect().unwrap();
+        let stmt = conn.prepare("SELECT 1").await.unwrap();
+        let before = closes();
+        drop(conn);
+        assert_eq!(
+            closes() - before,
+            0,
+            "closed while a statement still uses it"
+        );
+        drop(stmt);
+        assert_eq!(closes() - before, 1);
+    }
+
     #[tokio::test]
     pub async fn test_kek() {
         let temp_dir = tempfile::tempdir().unwrap();
diff --git a/libsql/src/local/impls.rs b/libsql/src/local/impls.rs
index b86405610e..10180c0321 100644
--- a/libsql/src/local/impls.rs
+++ b/libsql/src/local/impls.rs
@@ -105,12 +105,6 @@ impl Conn for LibsqlConnection {
     }
 }
 
-impl Drop for LibsqlConnection {
-    fn drop(&mut self) {
-        self.conn.disconnect()
-    }
-}
-
 pub(crate) struct LibsqlStmt(pub crate::local::Statement);
 
 #[async_trait::async_trait]