fix: prevent /new panic from SQLite pool contention#2995
fix: prevent /new panic from SQLite pool contention#2995BnjmnZmmrmn-parafin wants to merge 1 commit intotailcallhq:mainfrom
Conversation
9a0ffc5 to
44336db
Compare
|
verified by building locally, openning session and spamming /new repeatedly, seemed to work fine |
44336db to
e4a6201
Compare
e4a6201 to
5b27a90
Compare
|
Hi! I'm I would like to apply some automated changes to this pull request, but it looks like I don't have the necessary permissions to do so. To get this pull request into a mergeable state, please do one of the following two things:
|
3b02c24 to
c906bd5
Compare
VerificationTested locally against the fix branch using a debug build with Steps:
What we observed: Each Pool reinitializes cleanly on every |
c906bd5 to
2798089
Compare
Summary
Fix a panic on
/newcaused by SQLite lock contention whenForgeRepocreated a second connection pool pointing at the same.forge.dbfile while the previous pool was still alive.Context
Every time a user ran
/new,ForgeRepo::newcreated a brand-newDatabasePoolviaDatabasePool::try_from(...).unwrap(). The old pool — still held alive bytokio::spawn'd hydration tasks — would hold open connections to the same SQLite file. r2d2's pool builder validates connections at construction time with a 5-second timeout; after 5 retries the error bubbled back to the.unwrap(), crashing the process with:Changes
forge_repo:ForgeRepo::newnow accepts anArc<DatabasePool>instead of constructing its own.DatabasePoolandPoolConfigare re-exported from the crate root so callers don't need to reach into internal modules.forge_api:ForgeAPI::initcreates the pool once and passes it toForgeRepo::new. Return type changes fromSelftoResult<Self>— pool failure is now a clean error instead of a panic.forge_main:UI's factory closure bound changes fromFn(ForgeConfig) -> AtoFn(ForgeConfig) -> Result<A>.on_newpropagates the error via?instead of unwrapping.Key Implementation Details
The pool is created once per
forgeprocess insideForgeAPI::initusingConfigReader::base_path().join(".forge.db")— the same canonical path used byEnvironment::database_path(). On each/new, a newForgeRepois wired to the sameArc<DatabasePool>, so the underlying SQLite file is only ever opened once and there is no contention window.Testing
cargo build -p forge_repo -p forge_api -p forge_main # Manually verify: start forge, run /new multiple times, confirm no crash forge /new /new /new