From f560b31ee4f27e94cb935c833d07dc309ff4d3ba Mon Sep 17 00:00:00 2001 From: splitsec2 <35583321+splitsec2@users.noreply.github.com> Date: Thu, 22 Oct 2026 18:21:49 -0600 Subject: [PATCH] fix(db): wait up to 30 s for a busy writer instead of SQLite's 5 s default (#1539) The users, download history and activity view-state databases open their connections with SQLite's default busy timeout of 5 seconds. A start-up migration, the restart reconcile, or any statement that overlaps a backup or a second process raised "database is locked" after 5 seconds, and the reconcile finished only part of its work. One shared `DB_BUSY_TIMEOUT_SECONDS` (30) in `core/utils.py`, used by all three. The test reads SQLite's `busy_timeout` pragma back from each connection, so it doesn't need a real lock fight. I left `UserDB.initialize()` without a guard on purpose. With 30 seconds the wait covers a backup, and swallowing a schema failure there would hide a worker that can't run. Found in a Claude pass over 1.5.0. --- shelfmark/core/activity_view_state_service.py | 3 ++- shelfmark/core/download_history_service.py | 4 +-- shelfmark/core/user_db.py | 3 ++- shelfmark/core/utils.py | 5 ++++ tests/core/test_db_busy_timeout.py | 27 +++++++++++++++++++ 5 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 tests/core/test_db_busy_timeout.py diff --git a/shelfmark/core/activity_view_state_service.py b/shelfmark/core/activity_view_state_service.py index 3aeec4c8..8b914e95 100644 --- a/shelfmark/core/activity_view_state_service.py +++ b/shelfmark/core/activity_view_state_service.py @@ -7,6 +7,7 @@ import threading from typing import Any from shelfmark.core.request_helpers import now_utc_iso +from shelfmark.core.utils import DB_BUSY_TIMEOUT_SECONDS VALID_ACTIVITY_ITEM_TYPES = frozenset({"download", "request"}) ADMIN_VIEWER_SCOPE = "admin:shared" @@ -82,7 +83,7 @@ class ActivityViewStateService: self._lock = threading.Lock() def _connect(self) -> sqlite3.Connection: - conn = sqlite3.connect(self._db_path) + conn = sqlite3.connect(self._db_path, timeout=DB_BUSY_TIMEOUT_SECONDS) conn.row_factory = sqlite3.Row conn.execute("PRAGMA foreign_keys = ON") return conn diff --git a/shelfmark/core/download_history_service.py b/shelfmark/core/download_history_service.py index 86c8484e..97738e1e 100644 --- a/shelfmark/core/download_history_service.py +++ b/shelfmark/core/download_history_service.py @@ -16,7 +16,7 @@ from shelfmark.core.request_helpers import ( normalize_optional_text, now_utc_iso, ) -from shelfmark.core.utils import transform_cover_url +from shelfmark.core.utils import DB_BUSY_TIMEOUT_SECONDS, transform_cover_url logger = setup_logger(__name__) @@ -100,7 +100,7 @@ class DownloadHistoryService: self._lock = threading.Lock() def _connect(self) -> sqlite3.Connection: - conn = sqlite3.connect(self._db_path) + conn = sqlite3.connect(self._db_path, timeout=DB_BUSY_TIMEOUT_SECONDS) conn.row_factory = sqlite3.Row conn.execute("PRAGMA foreign_keys = ON") return conn diff --git a/shelfmark/core/user_db.py b/shelfmark/core/user_db.py index ad0734c2..4276dd0a 100644 --- a/shelfmark/core/user_db.py +++ b/shelfmark/core/user_db.py @@ -21,6 +21,7 @@ from shelfmark.core.request_validation import ( validate_request_level_payload, validate_status_transition, ) +from shelfmark.core.utils import DB_BUSY_TIMEOUT_SECONDS logger = setup_logger(__name__) @@ -194,7 +195,7 @@ class UserDB: return self._db_path def _connect(self) -> sqlite3.Connection: - conn = sqlite3.connect(self._db_path) + conn = sqlite3.connect(self._db_path, timeout=DB_BUSY_TIMEOUT_SECONDS) conn.row_factory = sqlite3.Row conn.execute("PRAGMA foreign_keys = ON") return conn diff --git a/shelfmark/core/utils.py b/shelfmark/core/utils.py index ab158b70..3c4e8342 100644 --- a/shelfmark/core/utils.py +++ b/shelfmark/core/utils.py @@ -18,6 +18,11 @@ if TYPE_CHECKING: logger = setup_logger(__name__) +# How long an app database connection waits for another writer before it raises. SQLite's +# own default is 5 s, which a start-up migration or a restart reconcile can outlast when a +# backup or a second process holds the file. +DB_BUSY_TIMEOUT_SECONDS = 30 + def normalize_http_url( url: str | None, diff --git a/tests/core/test_db_busy_timeout.py b/tests/core/test_db_busy_timeout.py new file mode 100644 index 00000000..5e2248eb --- /dev/null +++ b/tests/core/test_db_busy_timeout.py @@ -0,0 +1,27 @@ +"""The app databases wait out a busy writer instead of failing at SQLite's 5 s default.""" + +import pytest + +from shelfmark.core.activity_view_state_service import ActivityViewStateService +from shelfmark.core.download_history_service import DownloadHistoryService +from shelfmark.core.user_db import UserDB + +# SQLite's own default is 5000 ms. A start-up migration or a restart reconcile that meets a +# writer for a few seconds should wait, not raise. +_MIN_BUSY_TIMEOUT_MS = 15_000 + + +@pytest.mark.parametrize( + "service_cls", + [UserDB, DownloadHistoryService, ActivityViewStateService], +) +def test_app_database_connections_wait_for_a_busy_writer(tmp_path, service_cls): + service = service_cls(str(tmp_path / "users.db")) + + conn = service._connect() + try: + busy_timeout_ms = conn.execute("PRAGMA busy_timeout").fetchone()[0] + finally: + conn.close() + + assert busy_timeout_ms >= _MIN_BUSY_TIMEOUT_MS