fix(cache): close the connection the schema init opened

`with sqlite3.Connection` ends the transaction but keeps the file handle open,
so every cache class leaked one handle per instance until the garbage collector
happened to run. On Windows that makes the database file undeletable while the
process lives (WinError 32), which broke 30 cache tests in teardown and would
also block moving or clearing a cache at runtime.

All eight `with <connection>` sites now go through `closing_connection()`, and a
regression test asserts the schema init leaves no usable connection behind.
This commit is contained in:
Will Miao
2026-10-12 11:35:14 +08:00
parent 5fbac84910
commit fd40cd952b
7 changed files with 61 additions and 11 deletions
+2 -1
View File
@@ -8,6 +8,7 @@ import sqlite3
import time import time
from typing import Any, List, Optional from typing import Any, List, Optional
from ..utils.cache_db import closing_connection
from ..utils.cache_paths import get_cache_base_dir from ..utils.cache_paths import get_cache_base_dir
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@@ -127,7 +128,7 @@ class DownloadQueueService:
def _initialize_schema(self) -> None: def _initialize_schema(self) -> None:
if self._schema_initialized: if self._schema_initialized:
return return
with self._connect() as conn: with closing_connection(self._connect()) as conn:
conn.executescript(self._SCHEMA_TABLES) conn.executescript(self._SCHEMA_TABLES)
# Databases created by older versions lack # Databases created by older versions lack
@@ -11,6 +11,7 @@ import sqlite3
import time import time
from typing import Iterable, Mapping, Optional, Sequence from typing import Iterable, Mapping, Optional, Sequence
from ..utils.cache_db import closing_connection
from ..utils.cache_paths import get_cache_base_dir from ..utils.cache_paths import get_cache_base_dir
from .settings_manager import get_settings_manager from .settings_manager import get_settings_manager
@@ -100,7 +101,7 @@ class DownloadedVersionHistoryService:
def _initialize_schema(self) -> None: def _initialize_schema(self) -> None:
if self._schema_initialized: if self._schema_initialized:
return return
with self._connect() as conn: with closing_connection(self._connect()) as conn:
conn.executescript(self._SCHEMA) conn.executescript(self._SCHEMA)
conn.commit() conn.commit()
self._schema_initialized = True self._schema_initialized = True
+5 -4
View File
@@ -17,6 +17,7 @@ from typing import Any, Dict, Iterable, Iterator, List, Mapping, Optional, Seque
from .errors import RateLimitError, ResourceNotFoundError from .errors import RateLimitError, ResourceNotFoundError
from .settings_manager import get_settings_manager from .settings_manager import get_settings_manager
from ..utils.cache_db import closing_connection
from ..utils.cache_paths import CacheType, resolve_cache_path_with_migration from ..utils.cache_paths import CacheType, resolve_cache_path_with_migration
from ..utils.constants import MODEL_WEIGHT_FILE_TYPES from ..utils.constants import MODEL_WEIGHT_FILE_TYPES
from ..utils.civitai_utils import rewrite_preview_url from ..utils.civitai_utils import rewrite_preview_url
@@ -445,7 +446,7 @@ class ModelUpdateService:
if self._schema_initialized: if self._schema_initialized:
return return
try: try:
with self._connect() as conn: with closing_connection(self._connect()) as conn:
conn.execute("PRAGMA journal_mode=WAL") conn.execute("PRAGMA journal_mode=WAL")
conn.execute("PRAGMA foreign_keys = ON") conn.execute("PRAGMA foreign_keys = ON")
conn.executescript(self._SCHEMA) conn.executescript(self._SCHEMA)
@@ -484,7 +485,7 @@ class ModelUpdateService:
return return
try: try:
with sqlite3.connect(legacy_path, check_same_thread=False) as legacy_conn: with closing_connection(sqlite3.connect(legacy_path, check_same_thread=False)) as legacy_conn:
legacy_conn.row_factory = sqlite3.Row legacy_conn.row_factory = sqlite3.Row
status_rows = legacy_conn.execute( status_rows = legacy_conn.execute(
""" """
@@ -2315,7 +2316,7 @@ class ModelUpdateService:
status_rows: list[sqlite3.Row] = [] status_rows: list[sqlite3.Row] = []
version_rows: list[sqlite3.Row] = [] version_rows: list[sqlite3.Row] = []
with self._connect() as conn: with closing_connection(self._connect()) as conn:
for start in range(0, len(ids), self._SQLITE_MAX_VARIABLES): for start in range(0, len(ids), self._SQLITE_MAX_VARIABLES):
chunk = tuple(ids[start : start + self._SQLITE_MAX_VARIABLES]) chunk = tuple(ids[start : start + self._SQLITE_MAX_VARIABLES])
placeholders = ",".join("?" for _ in chunk) placeholders = ",".join("?" for _ in chunk)
@@ -2418,7 +2419,7 @@ class ModelUpdateService:
record.last_checked_at, record.last_checked_at,
1 if record.should_ignore_model else 0, 1 if record.should_ignore_model else 0,
) )
with self._connect() as conn: with closing_connection(self._connect()) as conn:
conn.execute( conn.execute(
""" """
INSERT INTO model_update_status ( INSERT INTO model_update_status (
+2 -2
View File
@@ -6,7 +6,7 @@ import threading
from dataclasses import dataclass, field from dataclasses import dataclass, field
from typing import Any, Dict, List, Mapping, Optional, Sequence, Set, Tuple from typing import Any, Dict, List, Mapping, Optional, Sequence, Set, Tuple
from ..utils.cache_db import connect_cache_db from ..utils.cache_db import closing_connection, connect_cache_db
from ..utils.cache_paths import CacheType, resolve_cache_path_with_migration from ..utils.cache_paths import CacheType, resolve_cache_path_with_migration
from ..utils.file_lock import exclusive_lock from ..utils.file_lock import exclusive_lock
from ..utils.root_paths import normalize_root_identity from ..utils.root_paths import normalize_root_identity
@@ -618,7 +618,7 @@ class PersistentModelCache:
if self._schema_initialized: if self._schema_initialized:
return return
try: try:
with self._connect() as conn: with closing_connection(self._connect()) as conn:
conn.execute("PRAGMA journal_mode=WAL") conn.execute("PRAGMA journal_mode=WAL")
conn.execute("PRAGMA foreign_keys = ON") conn.execute("PRAGMA foreign_keys = ON")
conn.executescript( conn.executescript(
+2 -2
View File
@@ -19,7 +19,7 @@ import threading
from dataclasses import dataclass, field from dataclasses import dataclass, field
from typing import Any, Dict, List, Optional, Set, Tuple from typing import Any, Dict, List, Optional, Set, Tuple
from ..utils.cache_db import connect_cache_db from ..utils.cache_db import closing_connection, connect_cache_db
from ..utils.cache_paths import CacheType, resolve_cache_path_with_migration from ..utils.cache_paths import CacheType, resolve_cache_path_with_migration
from ..utils.file_lock import exclusive_lock from ..utils.file_lock import exclusive_lock
@@ -457,7 +457,7 @@ class PersistentRecipeCache:
if self._schema_initialized: if self._schema_initialized:
return return
try: try:
with self._connect() as conn: with closing_connection(self._connect()) as conn:
conn.execute("PRAGMA journal_mode=WAL") conn.execute("PRAGMA journal_mode=WAL")
conn.execute("PRAGMA foreign_keys = ON") conn.execute("PRAGMA foreign_keys = ON")
conn.executescript( conn.executescript(
+18 -1
View File
@@ -14,7 +14,8 @@ the write path in WAL so readers are never blocked by a writer.
from __future__ import annotations from __future__ import annotations
import sqlite3 import sqlite3
from typing import Any from contextlib import contextmanager
from typing import Any, Iterator
# How long a connection waits for a competing writer before raising. # How long a connection waits for a competing writer before raising.
CONCURRENT_TIMEOUT_SECONDS = 30.0 CONCURRENT_TIMEOUT_SECONDS = 30.0
@@ -79,3 +80,19 @@ def connect_cache_db(
pass pass
return conn return conn
@contextmanager
def closing_connection(conn: sqlite3.Connection) -> Iterator[sqlite3.Connection]:
"""Yield *conn* and close it on exit.
``with sqlite3.Connection`` only ends the transaction: it leaves the file
handle open, and a live handle keeps the cache database undeletable on
Windows (``WinError 32``) until the garbage collector happens to run. Every
``with`` over a cache connection goes through here instead.
"""
try:
yield conn
finally:
conn.close()
+30
View File
@@ -724,3 +724,33 @@ class TestHasWorkflowColumn:
assert loaded is not None assert loaded is not None
assert loaded.raw_data[0]["id"] == "legacy-1" assert loaded.raw_data[0]["id"] == "legacy-1"
assert loaded.raw_data[0]["has_workflow"] is False assert loaded.raw_data[0]["has_workflow"] is False
def test_schema_initialization_closes_its_connection(tmp_path, monkeypatch):
"""The schema init must not leak a file handle.
``with sqlite3.Connection`` only ends the transaction: the handle stays open,
and on Windows a live handle keeps the cache database undeletable
(``WinError 32``) until the garbage collector happens to run -- which broke
every teardown that removes a temporary cache file.
"""
import sqlite3
from py.services import persistent_recipe_cache as module
opened = []
real_connect = sqlite3.connect
def tracking_connect(*args, **kwargs):
conn = real_connect(*args, **kwargs)
opened.append(conn)
return conn
monkeypatch.setattr(sqlite3, "connect", tracking_connect)
module.PersistentRecipeCache(db_path=str(tmp_path / "recipe.sqlite"))
assert opened, "initializing the schema should have opened a connection"
for conn in opened:
with pytest.raises(sqlite3.ProgrammingError):
conn.execute("SELECT 1")