SQLite: do not clear the rollback hook through a handle that is closing

[HuzaifaChaudary]

Fixes #38854

the crash

PruneCallback disposes a pooled connection, and the reporter's stack ends like this:

at Microsoft.Data.Sqlite.SqliteTransaction.RollbackExternal(Object userData)
at SQLitePCL.SQLite3Provider_e_sqlite3.rollback_hook_bridge_impl(IntPtr p)
at SQLitePCL.SQLite3Provider_e_sqlite3.NativeMethods.sqlite3_close_v2(IntPtr db)
at SQLitePCL.sqlite3.ReleaseHandle()
at System.Runtime.InteropServices.SafeHandle.Dispose()
at Microsoft.Data.Sqlite.SqliteConnectionInternal.Dispose()
at Microsoft.Data.Sqlite.SqliteConnectionPool.DisposeConnection(...)
at Microsoft.Data.Sqlite.SqliteConnectionPool.PruneCallback(Object _)

reading it from the bottom, the pool disposes the connection, sqlite3_close_v2 rolls back the transaction that was still open, and that fires the rollback hook. RollbackExternal then asks the same handle to clear the hook:

sqlite3_rollback_hook(_connection!.Handle, null, null);

the handle is inside ReleaseHandle at that moment, so DangerousAddRef throws ObjectDisposedException. it comes out of a native callback, so there is nothing to catch it, and on the prune timer that means an unhandled exception on a thread pool thread. the process is gone.

#38574 guarded SqliteConnectionInternal.Deactivate for the same underlying reason, but as the reporter says, that is not the frame that kills the process.

the fix

skip clearing the hook when the handle is already closed. it is being torn down with the connection anyway, so there is nothing to unregister:

var handle = _connection!.Handle;
if (handle is { IsClosed: false, IsInvalid: false })
{
    sqlite3_rollback_hook(handle, null, null);
}

that is the same shape as the guard #38574 put in Deactivate, so the two read alike.

i put the same guard in RollbackInternal, because the second stack in the report goes through there:

at Microsoft.Data.Sqlite.SqliteTransaction.RollbackInternal()
at Microsoft.EntityFrameworkCore.Storage.RelationalTransaction.DisposeAsync()

that one is caught by EF and only logged, so it is not fatal, but it is the same call into a dead handle. when the handle is gone there is no transaction left to roll back either, so the ROLLBACK; is skipped with it. happy to drop that half if you would rather keep this to the fatal path only, it is the one part of the change i could not write a test for.

reproducing it

the reporter's snippet, as a test:

using var connection = new SqliteConnection("Data Source=:memory:");
connection.Open();
using var transaction = connection.BeginTransaction();

connection.Handle!.Dispose();

Assert.True(transaction.ExternalRollback);

this does not fail on main, it takes the test host down with it:

Unhandled exception. System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'SQLitePCL.sqlite3'.
   at System.Runtime.InteropServices.SafeHandle.DangerousAddRef(Boolean& success)
   at Microsoft.Data.Sqlite.SqliteTransaction.RollbackExternal(Object userData) in SqliteTransaction.cs:line 239
   at SQLitePCL.SQLite3Provider_sqlite3.rollback_hook_bridge_impl(IntPtr p)
Exit code: 134

which is why the before column below is short. the run does not finish.

verification

suite	before	after
Microsoft.Data.Sqlite.sqlite3.Tests	exit 134, run aborted at 683, error: 1	exit 0, 701 total, 0 failed
Microsoft.Data.Sqlite.sqlite3mc.Tests	—	exit 0, 702 total, 0 failed
EFCore.Sqlite.Tests	—	exit 0, 890 total, 0 failed

one thing i did not change

SqliteConnectionPool.PruneCallback and SqliteConnectionFactory.PruneCallback are both Timer callbacks with no try. anything that throws inside either one still ends the process, so this fix removes the cause that was reported rather than the class of failure. i did not add a blanket catch because swallowing errors on a background timer is a call for you to make, not something to slip into a bug fix. tell me if you want it and i will add it here.

---

disclaimer: this contribution was prepared with the assistance of an ai agent. i reproduced the crash from the report first, confirmed the exact frame, and ran the three suites above locally before opening this.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.