Skip to content

fix(core): serialize database bootstrap across processes - #50344

Closed
Hona wants to merge 1 commit into
anomalyco:devfrom
Hona:db-bootstrap-race
Closed

fix(core): serialize database bootstrap across processes#50344
Hona wants to merge 1 commit into
anomalyco:devfrom
Hona:db-bootstrap-race

Conversation

@Hona

@Hona Hona commented Sep 21, 2026

Copy link
Copy Markdown
Member

Service.ensure keeps a spare CLI contender alive ("one candidate plus one lock probe"). The bootstrap lock in database.ts is an in-process Semaphore, so on a fresh database two processes can reach bootstrap together and one of them fails, is briefly registered as state: failed, and the desktop shows "Background service failed to start".

Dormant in practice: it needs an empty opencode.db and a contender that takes >5 s to register. It surfaces every run in bench:devex (fresh profile, source CLI booting under Bun).

sequenceDiagram
  participant A as contender A
  participant DB as opencode.db (empty)
  participant B as contender B
  A->>DB: PRAGMA journal_mode=WAL
  B->>DB: PRAGMA journal_mode=WAL
  Note over B,DB: ✗ "database is locked" (no busy_timeout yet)
  A->>DB: SELECT tables → empty
  B->>DB: SELECT tables → empty
  A->>DB: BEGIN … CREATE TABLE ×47 … COMMIT
  B->>DB: BEGIN … CREATE TABLE account_state
  Note over B,DB: ✗ "already exists" → state: failed
Loading

Two doors, both closed:

  • sqlite.bun.ts / sqlite.node.ts: PRAGMA busy_timeout = 5000 before the WAL switch (a fresh file's WAL switch takes an exclusive lock).
  • migration.ts: the emptiness check moves inside a BEGIN IMMEDIATE transaction, so the second process waits on the write lock and then finds the first one's tables and takes the applyOnly path.
const bootstrapped = yield* db.transaction(
  (tx) => Effect.gen(function* () {
    const tables = yield* tx.all(sql`SELECT name FROM sqlite_master …`)
    if (tables.some((t) => t.name === "session" || t.name === "session_v2")) return false
    …schema.up(tx)…
    return true
  }),
  { behavior: "immediate" },
)
if (!bootstrapped) return yield* applyOnly(db, migrations)

The new test stands in for two processes with two SqlClients over one file (no shared per-process lock), parks the first at its begin, lets the second bootstrap everything, then releases the first. It fails on the previous code with table … already exists.

Observed while running the desktop benches for #50327; that PR's bench:devex numbers were gathered with this fix applied.

@Hona
fix(core): serialize database bootstrap across processes
5cfb7a4 
Two service contenders bootstrapping one fresh database raced on the WAL switch and on CREATE TABLE. Set busy_timeout before enabling WAL and run the emptiness check inside an immediate transaction so the second process waits and then reuses the first one's schema.
Copilot AI lite review requested due to automatic review settings September 21, 2026 11:14
Copilot AI reviewed Sep 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This was referenced Sep 21, 2026
Merged
Merged
@opencode-agent opencode-agent Bot mentioned this pull request Oct 8, 2026
Open
@argszero argszero mentioned this pull request Oct 8, 2026
Closed
3 of 6 tasks
@argszero

argszero commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Something I hit while checking this against the repro in #53879, in case it is
useful before this lands.

The busy_timeout = 5000 line does not cover the WAL switch. SQLite does not
run the busy handler for a journal-mode change, so PRAGMA journal_mode = WAL
on a fresh file still returns SQLITE_BUSY straight away when another
connection is switching it. With this branch checked out, two processes opening
one new database and setting WAL lost a process in 7 of 8 runs, each at about
5 ms. Running the full bootstrap on this branch still lost a process in 4 of 6
runs, all database is locked, none already exists — so the
BEGIN IMMEDIATE change does close the CREATE TABLE door.

Retrying the pragma while it returns SQLITE_BUSY closes the other one, since
it converges as soon as either connection finishes the switch. I have that in
#53983 and left your change untouched, so it is a one-line rebase in either
order. Happy for it to fold into this PR instead if you would rather have the
race fixed in one place.

@thdxr
thdxr changed the base branch from v2 to dev October 10, 2026 19:12
@github-actions

github-actions Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Automated PR Cleanup

Thank you for contributing to opencode.

Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions.

This PR was closed because it matched the following cleanup criteria:

  • The PR was created more than 1 month ago
  • The PR had fewer than 2 positive reactions
  • Positive reactions are counted as thumbs-up, heart, celebration, or rocket reactions on the PR

PRs created within the last month are not affected by this cleanup.

If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate.

Thanks again for taking the time to contribute.

@github-actions github-actions Bot closed this Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

automated-pr-cleanup

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Hona @argszero