Skip to content

fix: prevent /new panic from SQLite pool contention#2995

Open
BnjmnZmmrmn-parafin wants to merge 1 commit intotailcallhq:mainfrom
BnjmnZmmrmn-parafin:benjaminzimmerman/fix-new-pool-panic
Open

fix: prevent /new panic from SQLite pool contention#2995
BnjmnZmmrmn-parafin wants to merge 1 commit intotailcallhq:mainfrom
BnjmnZmmrmn-parafin:benjaminzimmerman/fix-new-pool-panic

Conversation

@BnjmnZmmrmn-parafin
Copy link
Copy Markdown

Summary

Fix a panic on /new caused by SQLite lock contention when ForgeRepo created a second connection pool pointing at the same .forge.db file while the previous pool was still alive.

Context

Every time a user ran /new, ForgeRepo::new created a brand-new DatabasePool via DatabasePool::try_from(...).unwrap(). The old pool — still held alive by tokio::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:

ERROR: called `Result::unwrap()` on an `Err` value: Failed to create connection pool: timed out waiting for connection

Changes

  • forge_repo: ForgeRepo::new now accepts an Arc<DatabasePool> instead of constructing its own. DatabasePool and PoolConfig are re-exported from the crate root so callers don't need to reach into internal modules.
  • forge_api: ForgeAPI::init creates the pool once and passes it to ForgeRepo::new. Return type changes from Self to Result<Self> — pool failure is now a clean error instead of a panic.
  • forge_main: UI's factory closure bound changes from Fn(ForgeConfig) -> A to Fn(ForgeConfig) -> Result<A>. on_new propagates the error via ? instead of unwrapping.

Key Implementation Details

The pool is created once per forge process inside ForgeAPI::init using ConfigReader::base_path().join(".forge.db") — the same canonical path used by Environment::database_path(). On each /new, a new ForgeRepo is wired to the same Arc<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

@github-actions github-actions bot added type: feature Brand new functionality, features, pages, workflows, endpoints, etc. type: fix Iterations on existing features or infrastructure. labels Apr 14, 2026
@BnjmnZmmrmn-parafin BnjmnZmmrmn-parafin marked this pull request as draft April 14, 2026 07:05
@BnjmnZmmrmn-parafin BnjmnZmmrmn-parafin force-pushed the benjaminzimmerman/fix-new-pool-panic branch from 9a0ffc5 to 44336db Compare April 14, 2026 07:11
@BnjmnZmmrmn-parafin
Copy link
Copy Markdown
Author

verified by building locally, openning session and spamming /new repeatedly, seemed to work fine

@BnjmnZmmrmn-parafin BnjmnZmmrmn-parafin force-pushed the benjaminzimmerman/fix-new-pool-panic branch from 44336db to e4a6201 Compare April 14, 2026 07:20
@BnjmnZmmrmn-parafin BnjmnZmmrmn-parafin marked this pull request as ready for review April 14, 2026 07:21
@BnjmnZmmrmn-parafin BnjmnZmmrmn-parafin force-pushed the benjaminzimmerman/fix-new-pool-panic branch from e4a6201 to 5b27a90 Compare April 14, 2026 07:31
@autofix-ci
Copy link
Copy Markdown
Contributor

autofix-ci bot commented Apr 14, 2026

Hi! I'm autofix logoautofix.ci, a bot that automatically fixes trivial issues such as code formatting in pull requests.

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:

  1. Allow edits by maintainers for your pull request, and then re-trigger CI (for example by pushing a new commit).
  2. Manually fix the issues identified for your pull request (see the GitHub Actions output for details on what I would like to change).

@BnjmnZmmrmn-parafin BnjmnZmmrmn-parafin marked this pull request as draft April 14, 2026 07:50
@BnjmnZmmrmn-parafin BnjmnZmmrmn-parafin force-pushed the benjaminzimmerman/fix-new-pool-panic branch 4 times, most recently from 3b02c24 to c906bd5 Compare April 14, 2026 08:24
@BnjmnZmmrmn-parafin
Copy link
Copy Markdown
Author

Verification

Tested locally against the fix branch using a debug build with FORGE_LOG=debug.

Steps:

  1. Built the fix branch: cargo build -p forge_main
  2. Ran with debug logging: FORGE_LOG=debug ./target/debug/forge
  3. Ran /new repeatedly (10+ times back to back) in the forge prompt
  4. Tailed the log file at ~/forge/logs/forge.log.<date> to observe pool lifecycle

What we observed:

Each /new produced a clean Creating database poolcreated connection pool pair, completing in ~1ms with zero retries and zero contention errors:

{"timestamp":"   6.223s","level":"DEBUG","fields":{"message":"Creating database pool","database_path":"/Users/benjaminzimmerman/forge/.forge.db"}}
{"timestamp":"   6.224s","level":"DEBUG","fields":{"message":"created connection pool","database_path":"/Users/benjaminzimmerman/forge/.forge.db"}}
{"timestamp":"   7.236s","level":"DEBUG","fields":{"message":"Creating database pool","database_path":"/Users/benjaminzimmerman/forge/.forge.db"}}
{"timestamp":"   7.237s","level":"DEBUG","fields":{"message":"created connection pool","database_path":"/Users/benjaminzimmerman/forge/.forge.db"}}
{"timestamp":"   8.212s","level":"DEBUG","fields":{"message":"Creating database pool","database_path":"/Users/benjaminzimmerman/forge/.forge.db"}}
{"timestamp":"   8.212s","level":"DEBUG","fields":{"message":"created connection pool","database_path":"/Users/benjaminzimmerman/forge/.forge.db"}}
{"timestamp":"   9.153s","level":"DEBUG","fields":{"message":"Creating database pool","database_path":"/Users/benjaminzimmerman/forge/.forge.db"}}
{"timestamp":"   9.154s","level":"DEBUG","fields":{"message":"created connection pool","database_path":"/Users/benjaminzimmerman/forge/.forge.db"}}

Pool reinitializes cleanly on every /new with no contention, no retries, and no crash. Pre-fix this would panic on the 2nd or 3rd /new with Failed to create connection pool: timed out waiting for connection.

@BnjmnZmmrmn-parafin BnjmnZmmrmn-parafin force-pushed the benjaminzimmerman/fix-new-pool-panic branch from c906bd5 to 2798089 Compare April 14, 2026 08:33
@BnjmnZmmrmn-parafin BnjmnZmmrmn-parafin marked this pull request as ready for review April 14, 2026 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature Brand new functionality, features, pages, workflows, endpoints, etc. type: fix Iterations on existing features or infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants