handles.cs: Interlocked CAS for sqlite3 extra slot

[jamescater2]

handles.cs: Interlocked CAS for sqlite3 extra slot

Summary

Make sqlite3.GetOrCreateExtra and dispose_extra use Interlocked so concurrent hook registration and disposal can't race the backing extra field.

Context

sqlite3.extra is a lazily-created container that holds managed state for hooks — commit / rollback / update hooks, authoriser, progress handler, trace, profile, and the hook_handles that pin user delegates via GCHandle so SQLite's native function pointers remain valid.

#7 — GetOrCreateExtra TOCTOU race

Current code:

public T GetOrCreateExtra<T>(Func<T> f) where T : class, IDisposable
{
    if (extra != null) return (T)extra;
    else { var q = f(); extra = q; return q; }
}

Two threads racing first registration:

1. Both observe extra == null
2. Both call f() (each builds its own container)
3. Both write extra = q; one store wins, the other's q is orphaned

The orphaned q holds GCHandles for the user's delegates, but the db's extra slot now points at a different container. On close, only extra's container is disposed — the orphaned GCHandles leak. Worse, if both threads registered their callback via the native API, the later native registration overwrites the earlier one; SQLite still holds the overwritten function pointer, but the GCHandle protecting that delegate is no longer reachable from extra → use-after-free when SQLite next invokes it.

Fix: Volatile.Read for the fast path, Interlocked.CompareExchange for the slow path, dispose the losing candidate on CAS miss.

#8 — dispose_extra non-atomic read-and-null

Current code:

private void dispose_extra()
{
    if (extra != null) { extra.Dispose(); extra = null; }
}

Narrow but real: a finalizer-thread dispose_extra concurrent with a user-thread GetOrCreateExtra can reorder around the non-atomic read/write. Pairing dispose_extra with Interlocked.Exchange makes the read-and-null a single atomic step and composes cleanly with the CAS in #7.

Changes

File	Change
src/SQLitePCLRaw.core/handles.cs	Add using System.Threading; rewrite GetOrCreateExtra with CAS; rewrite dispose_extra with Interlocked.Exchange
src/common/tests_xunit.cs	test_get_or_create_extra_returns_same_instance_under_contention

Tests

test_get_or_create_extra_returns_same_instance_under_contention spawns max(4, ProcessorCount) threads that race GetOrCreateExtra, all unblocked by a ManualResetEventSlim, and asserts every caller gets the same instance back via Assert.Same. On unpatched code under contention it eventually observes the race and returns distinct instances; the assertion fails. Patched code passes deterministically.

Heads-up on CI

The lib project (SQLitePCLRaw.core.csproj) builds clean on main. The test project (tests.csproj) currently does not — src/tests/my_batteries_v2.cs on main references NativeLibrary.Load(name, assy, flags) and NativeLibrary.WHERE_PLAIN, neither of which exist in the current core surface. This is pre-existing test-infrastructure drift unrelated to this PR; #663's test-harness refactor resolves it as a side effect. CI on this PR will be red until that lands; the handles.cs changes themselves are correct on main.

Independence

This PR is independent of #663 at the library level — handles.cs is untouched by #663. The only overlap is the shared test-harness breakage noted above. The fix stands on its own merits.

[ericsink]

What's the context for these pull requests? Are they AI-generated?

[jamescater2]

I am building an in memory caching architecture and trying to run unit tests in parallel. We kept getting crashes which hinted at an issue with sqlite. We tracked down and fixed the first one but could not find the cause of the other three. I then asked Claude to audit sqlite and it came up with 8 critical errors. We fixed those errors and retested. That resolved two of the three errors. I spend most of today matching the 8 modifications to the crashes. After a lot of testing I identified the changes that actually fixed our issues. You saw the first PR last night. The second PR is just an update to the first one. The third PR is a fix for the threading issue. Many thanks, James.

[ericsink]

Thanks for the background. I'll be looking more closely at these soon.