Fix sqlite "database is locked" errors in scheduled tasks#18
Conversation
… errors Scheduled tasks, message history, and conversations all write to the same sqlite file from concurrent asyncio tasks/channels. The default rollback journal mode plus a short busy timeout meant contention (e.g. a scheduled task writing task_outputs while the web channel writes messages) eventually exceeded the timeout and raised "database is locked". Centralize connection creation in config.get_db_connection(), which turns on WAL (so readers don't block the writer) and raises busy_timeout to 30s.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a centralized get_db_connection helper in app/config.py to manage SQLite connections with WAL mode and busy timeouts, updating various database modules to use it. The review feedback points out a critical connection leak pattern across multiple files: using a sqlite3 connection as a context manager only manages transactions and does not close the connection, meaning exceptions will bypass the trailing conn.close() calls. To resolve this, the reviewer recommends wrapping connection lifecycles in try...finally blocks. Additionally, the reviewer suggests removing redundant PRAGMAs from the connection helper to avoid overhead and instead enabling WAL mode once during database initialization.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| conn = sqlite3.connect(db_path, timeout=timeout, isolation_level=isolation_level) | ||
| conn.execute("PRAGMA journal_mode=WAL") | ||
| conn.execute(f"PRAGMA busy_timeout={int(timeout * 1000)}") | ||
| return conn |
There was a problem hiding this comment.
Redundant and Potentially Problematic PRAGMAs on Every Connection
PRAGMA journal_mode=WALis persistent: Once WAL mode is enabled on a SQLite database file, it persists across connections and restarts. RunningPRAGMA journal_mode=WALon every connection open is not only redundant but can also cause significant lock contention, performance overhead, or even "database is locked" errors under high concurrency (as SQLite may attempt to recreate or verify the-waland-shmsidecar files).PRAGMA busy_timeoutis redundant: Thetimeoutparameter passed tosqlite3.connect(..., timeout=timeout)already sets the busy timeout internally (by callingsqlite3_busy_timeoutunder the hood). Setting it again via PRAGMA is unnecessary.
Recommendation:
Simplify get_db_connection to only open the connection. Enable WAL mode once during database initialization (e.g., in _init_tasks_db, _ensure_schema, and _ensure_db).
| conn = sqlite3.connect(db_path, timeout=timeout, isolation_level=isolation_level) | |
| conn.execute("PRAGMA journal_mode=WAL") | |
| conn.execute(f"PRAGMA busy_timeout={int(timeout * 1000)}") | |
| return conn | |
| return sqlite3.connect(db_path, timeout=timeout, isolation_level=isolation_level) |
|
|
||
| def _init_tasks_db(self): | ||
| with sqlite3.connect(APP_DB) as conn: | ||
| with get_db_connection() as conn: |
There was a problem hiding this comment.
Connection Leak and Transaction Management
In Python's sqlite3 module, using a connection as a context manager (with conn:) only manages transactions (committing on success, rolling back on exception). It does not close the connection.
Because conn.close() is called outside the with block (on line 53), any exception raised during table creation will bypass conn.close(), leaking the database connection. This can lead to file descriptor exhaustion and database locks.
Recommendation:
Use a try...finally block to guarantee that the connection is closed. Also, since PRAGMA journal_mode=WAL should only be run once during database initialization, execute it here.
def _init_tasks_db(self):
conn = get_db_connection()
try:
conn.execute("PRAGMA journal_mode=WAL")
with conn:
conn.execute("""
CREATE TABLE IF NOT EXISTS tasks (
...
)
""")
conn.execute("""
CREATE TABLE IF NOT EXISTS task_outputs (
...
)
""")
finally:
conn.close()|
|
||
| def remove_task(self, name: str): | ||
| with sqlite3.connect(APP_DB) as conn: | ||
| with get_db_connection() as conn: |
There was a problem hiding this comment.
Connection Leak on Exception
Using with conn: does not close the connection. If an exception occurs during conn.execute, the trailing conn.close() is bypassed, leaking the connection.
Recommendation:
Use a try...finally block to ensure the connection is closed, and let the connection context manager handle the transaction commit automatically:
def remove_task(self, name: str):
conn = get_db_connection()
try:
with conn:
conn.execute("DELETE FROM tasks WHERE name = ?", (name,))
finally:
conn.close()| def save_output(self, name: str, prompt: str, output: str, | ||
| status: str = "success", duration_secs: float = None): | ||
| with sqlite3.connect(APP_DB) as conn: | ||
| with get_db_connection() as conn: |
There was a problem hiding this comment.
Connection Leak on Exception
Using with conn: does not close the connection. If an exception occurs during conn.execute, the trailing conn.close() is bypassed, leaking the connection.
Recommendation:
Use a try...finally block to ensure the connection is closed, and let the connection context manager handle the transaction commit automatically:
def save_output(self, name: str, prompt: str, output: str,
status: str = "success", duration_secs: float = None):
conn = get_db_connection()
try:
with conn:
conn.execute("""
INSERT INTO task_outputs (name, prompt, output, status, duration_secs, timestamp)
VALUES (?, ?, ?, ?, ?, ?)
""", (name, prompt, output, status, duration_secs, datetime.now().isoformat()))
finally:
conn.close()|
|
||
| def get_output(self, name: str, num_entries: int = 5) -> list[dict]: | ||
| with sqlite3.connect(APP_DB) as conn: | ||
| with get_db_connection() as conn: |
There was a problem hiding this comment.
Connection Leak on Exception
Using with conn: does not close the connection. If an exception occurs during query execution, the trailing conn.close() is bypassed, leaking the connection.
Recommendation:
Use a try...finally block to ensure the connection is closed. Since this is a read-only query, a transaction context manager (with conn:) is not required:
def get_output(self, name: str, num_entries: int = 5) -> list[dict]:
conn = get_db_connection()
try:
rows = conn.execute("""
SELECT prompt, output, status, duration_secs, timestamp FROM task_outputs
WHERE name = ?
ORDER BY id DESC LIMIT ?
""", (name, num_entries)).fetchall()
finally:
conn.close()
return [{"prompt": p, "output": o, "status": s, "duration_secs": d, "timestamp": t}
for p, o, s, d, t in reversed(rows)]| next_due = original_next + (intervals_passed + 1) * interval | ||
| next_run = next_due.isoformat() | ||
| with sqlite3.connect(APP_DB) as conn: | ||
| with get_db_connection() as conn: |
There was a problem hiding this comment.
Connection Leak on Exception
Using with conn: does not close the connection. If an exception occurs during conn.execute, the trailing conn.close() is bypassed, leaking the connection.
Recommendation:
Use a try...finally block to ensure the connection is closed, and let the connection context manager handle the transaction commit automatically:
conn = get_db_connection()
try:
with conn:
conn.execute("""UPDATE tasks SET last_run = ?, next_run = ?, run_count = run_count + 1
WHERE name = ?""", (now.isoformat(), next_run, name))
finally:
conn.close()| # Phase 2: Migration — BEGIN IMMEDIATE for concurrency safety. | ||
| # The WHERE conversation_id IS NULL guard makes this re-entrant. | ||
| mconn = sqlite3.connect(self.db_path, isolation_level=None) | ||
| mconn = get_db_connection(self.db_path, isolation_level=None) |
There was a problem hiding this comment.
Explicitly Enable WAL Mode During Schema Initialization
Since PRAGMA journal_mode=WAL is removed from get_db_connection to avoid connection-time overhead and potential race conditions, we should explicitly enable WAL mode once during schema initialization.
Recommendation:
Execute PRAGMA journal_mode=WAL at the start of _ensure_schema (e.g., inside the _fk_conn block or on mconn before starting the transaction):
# Phase 1: DDL — idempotent CREATE TABLE/INDEX and optional ALTER TABLE
with _fk_conn(self.db_path) as conn:
conn.execute("PRAGMA journal_mode=WAL")
conn.execute("""
CREATE TABLE IF NOT EXISTS conversations (
...| try: | ||
| self.db_path.parent.mkdir(parents=True, exist_ok=True) | ||
| with sqlite3.connect(self.db_path) as conn: | ||
| with get_db_connection(self.db_path) as conn: |
There was a problem hiding this comment.
Connection Leak and Transaction Management
In Python's sqlite3 module, using a connection as a context manager (with conn:) only manages transactions (committing on success, rolling back on exception). It does not close the connection.
Because conn.close() is called outside the with block (on line 47), any exception raised during table creation or index creation will bypass conn.close(), leaking the database connection.
Recommendation:
Use a try...finally block to guarantee that the connection is closed. Also, since PRAGMA journal_mode=WAL should only be run once during database initialization, execute it here.
def _ensure_db(self):
try:
self.db_path.parent.mkdir(parents=True, exist_ok=True)
conn = get_db_connection(self.db_path)
try:
conn.execute("PRAGMA journal_mode=WAL")
with conn:
conn.execute("""
CREATE TABLE IF NOT EXISTS messages (
id INTEGER PRIMARY KEY AUTOINCREMENT,
channel text NOT NULL,
role TEXT NOT NULL,
content TEXT NOT NULL,
timestamp TEXT NOT NULL,
est_tokens INTEGER
)
""")
conn.execute("""
CREATE INDEX IF NOT EXISTS idx_messages_channel ON messages(channel)
""")
# Add conversation_id if missing
cols = {row[1] for row in conn.execute("PRAGMA table_info(messages)")}
if "conversation_id" not in cols:
conn.execute(
"ALTER TABLE messages ADD COLUMN conversation_id INTEGER"
)
conn.execute("""
CREATE INDEX IF NOT EXISTS idx_messages_conversation_id
ON messages(conversation_id)
""")
finally:
conn.close()
except sqlite3.Error as e:
log.error(f"Error creating message history database: {str(e)}")
raise| timestamp = datetime.now().isoformat() | ||
| est = _est_tokens(content) | ||
| with sqlite3.connect(self.db_path) as conn: | ||
| with get_db_connection(self.db_path) as conn: |
There was a problem hiding this comment.
Connection Leak on Exception
Using with conn: does not close the connection. If an exception occurs during conn.execute, the trailing conn.close() is bypassed, leaking the connection.
Recommendation:
Use a try...finally block to ensure the connection is closed, and let the connection context manager handle the transaction commit automatically:
conn = get_db_connection(self.db_path)
try:
with conn:
conn.execute(
"INSERT INTO messages (channel, role, content, timestamp, est_tokens, conversation_id) "
"VALUES (?, ?, ?, ?, ?, ?)",
(self.channel, role, content, timestamp, est, conversation_id),
)
finally:
conn.close()|
|
||
| def get_history(self, limit: int = 100) -> list[dict]: | ||
| with sqlite3.connect(self.db_path) as conn: | ||
| with get_db_connection(self.db_path) as conn: |
There was a problem hiding this comment.
Connection Leak on Exception
Using with conn: does not close the connection. If an exception occurs during query execution, the trailing conn.close() is bypassed, leaking the connection.
Recommendation:
Use a try...finally block to ensure the connection is closed. Since this is a read-only query, a transaction context manager (with conn:) is not required:
def get_history(self, limit: int = 100) -> list[dict]:
conn = get_db_connection(self.db_path)
try:
rows = conn.execute("""SELECT role, content FROM messages
WHERE channel = ?
ORDER BY id DESC LIMIT ?""", (self.channel, limit)).fetchall()
finally:
conn.close()
return [{"role": row[0], "content": row[1]} for row in reversed(rows)]- get_db_connection() no longer sets PRAGMA journal_mode=WAL or busy_timeout on every connection: WAL is persisted in the db file so it only needs enabling once per store's schema-init, and busy_timeout is already set by sqlite3.connect's timeout param. - Replaced `with conn: ... ; conn.close()` patterns in scheduled_tasks.py and message_history.py with try/finally so an exception during the transaction can't skip closing the connection.
* fix: enable WAL mode + busy_timeout for shared app.db to prevent lock errors Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Summary
app.dbfile from concurrent asyncio tasks/channels, but each opened rawsqlite3.connect()with the default rollback-journal mode and a 5s busy timeout, so growing contention (e.g. the 30s scheduled-task poll loop colliding with web channel conversation writes) eventually exceeded the timeout and raiseddatabase is locked.get_db_connection()inapp/config.py, a shared connection helper that enablesPRAGMA journal_mode=WAL(readers no longer block the writer) and raisesbusy_timeoutto 30s so genuine writer/writer collisions retry instead of failing almost immediately.scheduled_tasks.py,message_history.py, andconversations.pyto open allAPP_DBconnections through this helper.Test plan
uv run --python 3.13 pytest tests/— 279 passedPRAGMA journal_modereportswalafterScheduledTasksinitializes the db, confirming the setting persists at the file level for all consumershttps://claude.ai/code/session_018SQpEZ8aJAfisz1corx6fC
Generated by Claude Code