Fix site boot failure when the SQLite database is left in WAL mode - #4819
Merged
Merged
Conversation
Convert the database out of WAL on every Playground start, not just after an import, and wait for a SIGKILLed server process to actually exit before returning from stop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
📊 Performance Test ResultsComparing b461fe3 vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
1 task
wojtekn
added a commit
that referenced
this pull request
Sep 23, 2026
## Related issues - Fixes #4843 ## How AI was used in this PR Claude Code traced the report through the bundled SQLite driver and Studio's subprocess paths, and wrote the change. I reviewed it and pushed back on an earlier, larger version — see the scope note below. ## Proposed Changes #4819 fixed `Cannot escape data without an active database connection` by converting a site's SQLite database out of WAL journal mode at site start, but gated that to the Playground runtime on the reasoning that Native PHP "runs the platform's own SQLite against real OS locks." That gate leaves the default configuration unprotected: `getSiteRuntime()` returns Native PHP when a site has no explicit runtime, so **most sites never run the conversion**. Measured on trunk, same site, same database — the runtime alone decides: | Runtime | `journal_mode` after start | | --- | --- | | Playground (`sandbox`) | `delete` | | Native PHP (default) | `wal` | Real OS locks avoid PHP-WASM's emulated-lock problem, but they do not make WAL safe here, because the contention is between *processes*: Studio opens one site's database from several at once — an export spawns a WP-CLI process per table while the site server keeps serving — and those connections contend for WAL's `-shm` shared-memory index. Past the driver's 10s busy timeout the connection fails, and the driver swallows it into the misleading fatal above. That matches the report's observation that retried pushes failed at a *different* internal step each time: per-connection contention, not one stuck lock. This removes the runtime gate so both runtimes get the conversion that #4819 already proved out. ### Scope note for reviewers An earlier version of this PR also pinned `SQLITE_JOURNAL_MODE=DELETE` in `wp-config.php` so subprocesses could never re-enter WAL. **That was dropped.** WAL is the upstream default for good reason — WordPress/sqlite-database-integration#405 measured 3.2×–5.8× higher throughput and a p99 drop from 59ms to 4ms under concurrent load — and pinning DELETE globally would forfeit that on every platform to address a problem only reported on Windows. So this PR deliberately does **not** settle whether Studio should use WAL. It only makes the existing behavior consistent across runtimes. The underlying question — whether Studio's multi-process access pattern is better fixed by serializing DB access, raising the busy timeout, or a Windows-scoped change — is worth answering upstream, where WordPress/sqlite-database-integration#443 (`SET` no longer taking a write lock, opened from STU-1821) fixed the last contention bug of this family.⚠️ The fatal in #4843 is Windows-specific and I could not reproduce it on macOS. This removes the mechanism on the runtime that was missing it; **confirming the crash is gone needs a Windows push.** ## Testing Instructions Check a site's mode (read-only, safe while running): ``` node -e "const{DatabaseSync}=require('node:sqlite');const d=new DatabaseSync(process.argv[1],{readOnly:true});console.log(d.prepare('PRAGMA journal_mode').get().journal_mode);d.close();" ~/Studio/<site>/wp-content/database/.ht.sqlite ``` 1. On `trunk`, start a **native** (default) site and confirm it reads `wal`, with `.ht.sqlite-wal` / `.ht.sqlite-shm` present. 2. On this branch, `npm run cli:build`, stop the site, then start it again. 3. It should now read `delete`, with both sidecar files gone. 4. Repeat with `--runtime sandbox` to confirm Playground behavior is unchanged. Verified on macOS against one site switched between both runtimes: Playground `delete` (unchanged), native `wal` → `delete`. Forcing a stopped database to WAL and starting it confirmed the conversion is what moves it. Note when testing in dev mode: stop the site first (the conversion intentionally swallows errors if the database is locked, so a running site silently shows no change), quit any running `npm start` (it uses the same dev CLI build), and let `npm run cli:build` finish before launching — the CLI daemon loads its bundle at startup. ## Pre-merge Checklist - [x] Have you checked for TypeScript, React or other console errors? Lint, `npm run typecheck`, and the affected unit tests pass. The test asserting the native runtime was skipped is inverted to assert the conversion now runs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues
How AI was used in this PR
Claude Code diagnosed the crash from the issue's stack trace, confirmed the WAL theory against real site databases and by polling journal mode during a live start, wrote the fix and its tests, and verified that each new test fails against the pre-fix code. I reviewed the analysis and the change myself.
Proposed Changes
Sites could stop booting entirely, failing with a fatal that names the wrong subsystem:
The real failure is the database connection, not escaping. Since sqlite-database-integration v3.0.0 the driver connects in WAL journal mode by default, and WAL is persisted in the database file header — so any boot that writes to the database leaves it in WAL, not just an import.
WAL needs a shared-memory index (the
-shmfile), which SQLite coordinates through file locks. SQLite's WAL support is compiled into PHP-WASM, but the locks underneath it are not real:@php-wasm/nodeemulates them on the host, with separate implementations per platform. The POSIX one delegates toflock/fcntl, while the Windows one maps onto realLockFileEx/UnlockFileExcalls that are released only during orderly cleanup. A lock outliving its process — or contention between the outgoing and incoming server across a restart — makes reopening a WAL database fail intermittently with "database is locked".That failure is then swallowed:
wpdb::bail()only callswp_die()whenshow_errorsis on, and it defaults to off. WordPress carries on booting with$wpdb->dbhstill null and crashes several frames later insideesc_sql(), where the SQLite driver's_real_escape()throws instead of degrading the way core's does. That mismatch is what turns a recoverable connection failure into a hard fatal, and why the reported error points nowhere near the cause.Studio already had the conversion that prevents this, but it only ran after an import — written when imports really were the only way a database ended up in WAL. The v3.0.0 default silently widened the problem to every site without widening the guard. This runs it on every Playground start, which explains the "worked for two days, then failed permanently" reports: normal use flipped the database to WAL, and the header kept it there.
Separately, stopping a server could return while the process was still alive. When the graceful stop timed out, the SIGKILL fallback returned without waiting for the process to actually exit, while still reporting "WordPress server stopped". Callers restart immediately afterwards, and on Windows the dying process's handle on the SQLite file made that restart fail to connect. This is the likely cause of the intermittent
CLI E2E Tests on windowsfailures onapplies a changed admin password to WordPress, which stops and restarts a site back to back.Native PHP is deliberately left alone: it runs the platform's own SQLite against real OS locks, with no emulation layer in between, so WAL works there as it does anywhere else. The conversion is gated to the Playground runtime for that reason.
Testing Instructions
The underlying failure is Windows-only and intermittent, so CI on this PR is the real test — particularly the
CLI E2E Tests on windowsjob.To verify the conversion on any platform:
node -e "const{DatabaseSync}=require('node:sqlite');const d=new DatabaseSync(process.env.HOME+'/Studio/<site>/wp-content/database/.ht.sqlite',{readOnly:true});console.log(d.prepare('PRAGMA journal_mode').get().journal_mode)"/?rest_route=/wp/v2/posts.Pre-merge Checklist
🤖 Generated with Claude Code