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
Comment thread .forge/FORGE_PR_DESCRIPTION.md Outdated
@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
Comment thread crates/forge_main/src/main.rs Outdated
@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
@BnjmnZmmrmn-parafin
Copy link
Copy Markdown
Author

allowed edits by maintainers so lint could run! sorry not sure why this default off

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