fix(kanban): close half-open tracked connection when busy_timeout PRAGMA fails
_sqlite_connect opened a connection via connect_tracked and then ran the busy_timeout PRAGMA; if that raised, the half-open connection was abandoned — leaking its fd AND leaving a stale entry in the sqlite_safe_read live-connection registry (which only clears on close), permanently blocking byte-level probes of the kanban database. Close before re-raising. Salvaged from PR #96290 (kanban slice) with regression test.
This commit is contained in:
@@ -0,0 +1,2 @@
|
||||
Hjharris1
|
||||
# PR #96290 salvage
|
||||
+15
-4
@@ -1589,10 +1589,21 @@ def _sqlite_connect(path: Path) -> sqlite3.Connection:
|
||||
isolation_level=None,
|
||||
timeout=busy_timeout_ms / 1000.0,
|
||||
)
|
||||
# ``sqlite3.connect(timeout=...)`` normally maps to busy_timeout, but set
|
||||
# the PRAGMA explicitly so it is observable and survives future wrapper
|
||||
# changes. Parameter binding is not supported for PRAGMA assignments.
|
||||
conn.execute(f"PRAGMA busy_timeout={busy_timeout_ms}")
|
||||
try:
|
||||
# ``sqlite3.connect(timeout=...)`` normally maps to busy_timeout, but set
|
||||
# the PRAGMA explicitly so it is observable and survives future wrapper
|
||||
# changes. Parameter binding is not supported for PRAGMA assignments.
|
||||
conn.execute(f"PRAGMA busy_timeout={busy_timeout_ms}")
|
||||
except BaseException:
|
||||
# A half-open connection abandoned here would leak its fd AND leave a
|
||||
# stale entry in the connect_tracked live-connection registry (which
|
||||
# only clears on close), permanently blocking byte-level probes of
|
||||
# this database file. Close before re-raising.
|
||||
try:
|
||||
conn.close()
|
||||
except Exception:
|
||||
pass
|
||||
raise
|
||||
return conn
|
||||
|
||||
|
||||
|
||||
@@ -1007,6 +1007,39 @@ def test_connect_works_when_wal_is_silently_refused(tmp_path, monkeypatch, caplo
|
||||
)
|
||||
|
||||
|
||||
def test_sqlite_connect_closes_tracked_conn_on_setup_failure(tmp_path, monkeypatch):
|
||||
"""A PRAGMA failure after connect must not abandon a tracked kanban fd."""
|
||||
from hermes_cli import sqlite_safe_read
|
||||
|
||||
db_path = tmp_path / "kanban.db"
|
||||
real_connect = sqlite3.connect
|
||||
opened = []
|
||||
|
||||
class _BusyTimeoutFailure(sqlite3.Connection):
|
||||
def execute(self, sql, *args, **kwargs): # type: ignore[override]
|
||||
if str(sql).startswith("PRAGMA busy_timeout="):
|
||||
raise sqlite3.OperationalError("simulated setup failure")
|
||||
return super().execute(sql, *args, **kwargs)
|
||||
|
||||
def failing_connect(*args, **kwargs):
|
||||
kwargs.pop("factory", None)
|
||||
conn = real_connect(*args, factory=_BusyTimeoutFailure, **kwargs)
|
||||
opened.append(conn)
|
||||
return conn
|
||||
|
||||
key = sqlite_safe_read._key(db_path)
|
||||
with sqlite_safe_read._live_lock:
|
||||
before = sqlite_safe_read._live_connections.get(key, 0)
|
||||
monkeypatch.setattr(kb.sqlite3, "connect", failing_connect)
|
||||
|
||||
with pytest.raises(sqlite3.OperationalError, match="simulated setup failure"):
|
||||
kb._sqlite_connect(db_path)
|
||||
|
||||
with sqlite_safe_read._live_lock:
|
||||
after = sqlite_safe_read._live_connections.get(key, 0)
|
||||
assert after == before
|
||||
|
||||
|
||||
def test_unlink_tasks_triggers_recompute_ready(kanban_home):
|
||||
"""Regression test for issue #22459.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user