fix(sqlite): reject work after shutdown close

[NathanFlurry]

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• fix(sqlite): reject work after shutdown close #4903 👈 (View in Graphite)
• fix(rivetkit): drain shutdown work before sleep #4902 : 1 other dependent PR (#4904 )
• feat(sqlite): wire remote sqlite execute handlers #4897
• test(serverless): avoid pinning header validation order #4896
• chore(driver-test-runner): add native/wasm runtime matrix to skill #4884 : 3 other dependent PRs (#4892 , #4894 , #4901 )
• chroe: rename rivetkit-sqlite to depot-client #4883
• fix(rivetkit-wasm): fix mem leaks #4880
• chore(rivetkit): wasm support #4860
• feat(sqlite): add cold read benchmarks and simplify optimizations #4857
• feat(sqlite): benchmark cold reads #4848 : 1 other dependent PR (#4864 )
• docs(actors): document manual lifecycle controls #4847
• test(sqlite): expand db stress coverage #4846 : 1 other dependent PR (#4849 )
• test(raw-websocket): add serverless smoke coverage #4845
• feat(serverless): configure drain grace period #4844
• fix(publish): enable frontend build for engine target and use turbo build instead of turbo build:engine #4843
• feat(rivetkit): expose conn handle in onWebSocket context #4842
• fix(rivetkit): normalize waitUntil/keepAwake promises to null to avoid serde undefined error #4841
• fix(rivetkit): remove requirement for token #4839 : 1 other dependent PR (#4840 )
• fix(pegboard-envoy): allow sqlite requests across lifecycle states #4838
• fix(pegboard): route scoped actor key reads by runner dc #4837
• fix(sqlite-storage): let sqlite open take over stale owners #4834
• fix(engine-tests): parallelize multi-DC startup + enable 6 multi-DC tests #4833
• fix(api): honor cursor in actor_ids list path #4832
• fix(api): silently filter invalid IDs from actor_ids query instead of 400 #4831
• fix(api-peer): make actors_delete idempotent for already-destroyed actors #4830
• fix(pegboard): use Iterator streaming mode in list_names so limit param is honored #4829
• chore(engine-tests): convert envoy tests to Sleep crash policy + triage ignored #4828
• feat(engine-tests): wire envoy actor names through prepopulate_actor_names #4827
• chore(depot): fault injection tests #4868
• feat(sqlite): pitr & forking #4859 : 1 other dependent PR (#4863 )
• chore(sqlite): stateless storage refactor #4856
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Review: fix(sqlite): reject work after shutdown close

Adds a shutdown_closed: Arc<AtomicBool> guard to SqliteDb so any operation attempted after cleanup() returns SqliteRuntimeError::Closed instead of racing against a partially-closed connection.

Correctness

The three critical cases in open() (local-native path) are all covered:

1. Shutdown before open_lock — ensure_not_shutdown_closed() fires before the lock is acquired; error returned immediately.
2. Shutdown while waiting for open_lock — cleanup() sets the flag before blocking on open_lock, so once open() acquires the lock the re-check inside catches it.
3. Shutdown races open_database_from_envoy() — the post-open check closes the freshly-opened handle and returns Closed before storing it into self.db.

The cleanup() ordering (set flag → acquire open_lock → flush → close) correctly prevents a newly-opened handle from being left behind.

The Arc wrapping on AtomicBool is justified because SqliteDb: Clone — clones must share the same shutdown state.

Minor suggestions

1. Ordering: SeqCst can be relaxed to Acquire/Release

A Release store in cleanup() paired with Acquire loads in ensure_not_shutdown_closed() and the post-open check provides the same happens-before guarantee as SeqCst, at lower cost on ARM. Not a blocker.

2. cleanup() #[cfg] split is slightly awkward

The open_lock guard is declared in one #[cfg(feature = "sqlite-local")] attribute and the body in a second block. Combining them makes the drop scope explicit. Correctness is unaffected; readability nit only.

pub(crate) async fn cleanup(&self) -> Result<()> {
    self.shutdown_closed.store(true, Ordering::SeqCst);
    #[cfg(feature = "sqlite-local")]
    {
        let _open_guard = self.open_lock.lock().await;
        self.stop_preload_hint_flush_task();
        self.flush_preload_hints_before_close().await;
    }
    self.close().await
}

Coverage of public API

All async write paths (get_pages, commit, open, exec, query, run, execute, execute_write) have the guard added. The pub(crate) helpers (query_rows_cbor, exec_rows_cbor, run_cbor, execute_rows_cbor) delegate to the guarded public methods so they are covered transitively. close() is intentionally unguarded (idempotent, called by cleanup() itself). No gaps found.

Verdict

The fix is correct and well-scoped. The two nits above (memory ordering, #[cfg] style) are minor. LGTM.