fix(doctor): name the processes holding a WAL database that journal_mode=delete never converted
`hermes doctor` already warns `<db> is in WAL mode despite database.journal_mode=delete` (#112240) and tells the operator to stop every Hermes process before the offline `PRAGMA journal_mode=DELETE`. It did not say WHICH processes: the output was identical with a gateway holding the file and with nobody holding it, so the operator had to guess between systemd/launchd/s6/Compose/Desktop owners. Under that warning doctor now runs the same fail-closed holder scan maintenance admission uses (`hermes_state_holders.foreign_state_db_holders`) and prints one line per holder PID with its command and the db/-wal/-shm files it holds. A partial or unavailable scan (sentinel or uninspectable entries, Windows) is reported as "cannot prove the database is quiet", never as an all-clear; an empty complete scan says the conversion can run now. Fixes #111729 (the warning half landed in #112240; this is the holder-naming half). Co-authored-by: John Paul Soliva <soliva.johnpaul@icloud.com>
This commit is contained in:
@@ -89,6 +89,37 @@ def _format_db_size(db_path: Path) -> str:
|
||||
return "size unknown"
|
||||
|
||||
|
||||
def _report_database_holders(name: str, db_path: Path) -> None:
|
||||
"""Name the processes holding ``db_path`` (or a WAL sidecar) so the operator knows what to stop before the
|
||||
offline journal-mode conversion; a partial or unavailable scan is reported as "cannot prove quiet", never as
|
||||
an all-clear (the scan is the same fail-closed authority repair/VACUUM/checkpoint admission uses)."""
|
||||
from hermes_state_holders import _read_proc_argv, foreign_state_db_holders, psutil
|
||||
if sys.platform == "win32":
|
||||
check_warn(f"{name}: cannot prove the database is quiet", "(holder scan is unavailable on Windows)")
|
||||
return
|
||||
unknown: list[str] = []
|
||||
by_pid: dict[int, set[str]] = {}
|
||||
for pid, target in foreign_state_db_holders(db_path):
|
||||
if pid <= 0 or target.startswith("uninspectable"):
|
||||
unknown.append(target)
|
||||
else:
|
||||
by_pid.setdefault(pid, set()).add(Path(target.removesuffix(" (deleted)")).name)
|
||||
for pid in sorted(by_pid):
|
||||
argv = _read_proc_argv(pid) # /proc only; macOS holders come from psutil
|
||||
if argv is None and psutil is not None:
|
||||
try:
|
||||
argv = psutil.Process(pid).cmdline() or None
|
||||
except Exception:
|
||||
argv = None
|
||||
who = " ".join([Path(argv[0]).name, *argv[1:]])[:80] if argv else "command line unavailable"
|
||||
check_info(f"{name} is held by PID {pid} ({who}): {', '.join(sorted(by_pid[pid]))}")
|
||||
if unknown:
|
||||
check_warn(f"{name}: cannot prove the database is quiet",
|
||||
f"(holder scan incomplete: {unknown[0][:120]}" + (f"; +{len(unknown) - 1} more" if len(unknown) > 1 else "") + ")")
|
||||
elif not by_pid:
|
||||
check_info(f"{name}: no other process holds it right now — the offline conversion can run")
|
||||
|
||||
|
||||
def _report_database_journal_modes(hermes_home: Path | None = None, version_info: tuple[int, ...] | None = None) -> None:
|
||||
"""List each database's journal mode; warn on WAL under a vulnerable SQLite, and on a configured
|
||||
``database.journal_mode: delete`` that never took effect."""
|
||||
@@ -122,6 +153,7 @@ def _report_database_journal_modes(hermes_home: Path | None = None, version_info
|
||||
+ ("; also exposed to the WAL-reset bug" if vulnerable else "")
|
||||
+ ". Stop every Hermes process for this profile, then run a one-time offline "
|
||||
"'PRAGMA journal_mode=DELETE' on the file)")
|
||||
_report_database_holders(name, path)
|
||||
elif error is not None:
|
||||
if vulnerable:
|
||||
check_warn(f"{name}: journal mode could not be read", f"({error}; cannot rule out WAL exposure)")
|
||||
|
||||
@@ -10,6 +10,8 @@ read-only engine open creates -wal/-shm sidecar files next to a WAL database.
|
||||
import os
|
||||
import re
|
||||
import sqlite3
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -461,6 +463,43 @@ class TestConfiguredDeleteNeverApplied:
|
||||
assert "state.db: WAL journal mode" not in out
|
||||
assert ("To clear the exposure:" in out) is exposed
|
||||
|
||||
@pytest.mark.skipif(sys.platform == "win32", reason="holder scan has no Windows backend")
|
||||
def test_wal_db_under_configured_delete_names_its_holders(self, tmp_path, capsys, monkeypatch):
|
||||
# The offline conversion needs the file quiet, so doctor must say WHICH process to stop — a
|
||||
# subprocess holding a real connection is named by PID; the doctor process itself is not a holder.
|
||||
db = tmp_path / "state.db"
|
||||
_make_db(db, journal_mode="WAL")
|
||||
monkeypatch.setattr("hermes_state_wal.resolve_journal_mode", lambda: "delete")
|
||||
holder = subprocess.Popen(
|
||||
[sys.executable, "-c",
|
||||
"import sqlite3, sys; c = sqlite3.connect(sys.argv[1]); c.execute('SELECT count(*) FROM t'); "
|
||||
"print('ready', flush=True); sys.stdin.readline()", str(db)],
|
||||
stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True,
|
||||
)
|
||||
try:
|
||||
assert holder.stdout.readline().strip() == "ready"
|
||||
doctor_platform._report_database_journal_modes(tmp_path, (3, 51, 3))
|
||||
finally:
|
||||
holder.stdin.write("\n")
|
||||
holder.stdin.flush()
|
||||
holder.wait(timeout=30)
|
||||
|
||||
out = capsys.readouterr().out
|
||||
assert f"state.db is held by PID {holder.pid}" in out and "state.db" in out.split("held by PID")[1]
|
||||
assert "no other process holds it" not in out and "cannot prove" not in out
|
||||
|
||||
def test_partial_holder_scan_is_never_an_all_clear(self, tmp_path, capsys, monkeypatch):
|
||||
_make_db(tmp_path / "state.db", journal_mode="WAL")
|
||||
monkeypatch.setattr("hermes_state_wal.resolve_journal_mode", lambda: "delete")
|
||||
monkeypatch.setattr("hermes_state_holders.foreign_state_db_holders",
|
||||
lambda path: [(-1, "open-file scan unavailable")])
|
||||
|
||||
doctor_platform._report_database_journal_modes(tmp_path, (3, 51, 3))
|
||||
|
||||
out = capsys.readouterr().out
|
||||
assert "cannot prove the database is quiet" in out and "open-file scan unavailable" in out
|
||||
assert "no other process holds it" not in out and "held by PID" not in out
|
||||
|
||||
def test_configured_wal_keeps_the_informational_line(self, tmp_path, capsys, monkeypatch):
|
||||
_make_db(tmp_path / "state.db", journal_mode="WAL")
|
||||
monkeypatch.setattr("hermes_state_wal.resolve_journal_mode", lambda: "wal")
|
||||
|
||||
@@ -127,7 +127,10 @@ mode is not live-downgraded when you set `journal_mode: delete` (a downgrade
|
||||
under open connections can corrupt it). `hermes doctor` warns
|
||||
`<db> is in WAL mode despite database.journal_mode=delete` until you stop
|
||||
every Hermes process for the profile and run a one-time offline
|
||||
`PRAGMA journal_mode=DELETE` on the file.
|
||||
`PRAGMA journal_mode=DELETE` on the file. Under that warning it names the
|
||||
processes currently holding the database (`<db> is held by PID <n> (<command>)`)
|
||||
so you know what to stop; when the holder scan is partial or unavailable it says
|
||||
`cannot prove the database is quiet` instead of giving an all-clear.
|
||||
|
||||
## Environment Variable Substitution
|
||||
|
||||
|
||||
Reference in New Issue
Block a user