From 1dbcdcd7847223e61186e596c8468d4262cbc40c Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Tue, 15 Sep 2026 18:34:21 -0700 Subject: [PATCH] fix(db): copy the legacy database inside the process lock --- app/database/db.py | 11 +++--- tests-unit/app_test/test_db_init_locking.py | 37 +++++++++++++++++++++ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/app/database/db.py b/app/database/db.py index 053df2883..008e01b8e 100644 --- a/app/database/db.py +++ b/app/database/db.py @@ -115,8 +115,6 @@ def prepare_file_db_path(db_path): if db_dir: os.makedirs(db_dir, exist_ok=True) - copy_legacy_default_db(db_path) - _db_lock = None @@ -183,14 +181,13 @@ def _init_file_db(db_url): """Initialize a file-backed SQLite database using Alembic migrations.""" db_path = get_db_path() prepare_file_db_path(db_path) - db_exists = os.path.exists(db_path) - # Lock BEFORE any migration work — deliberately diverging from upstream master, whose - # "it would block Alembic" rationale is false (the lock guards a separate `.lock` - # file). Only this order makes revision inspection, backup, upgrade and the failure-path - # restore mutually exclusive between processes. + # The lock lives beside the database, so its parent directory must exist first. + # All database reads and writes, including the legacy import, run under the lock. _acquire_file_lock(db_path) try: + copy_legacy_default_db(db_path) + db_exists = os.path.exists(db_path) _migrate_and_bind(db_url, db_path, db_exists) except Exception: _db_lock.release() diff --git a/tests-unit/app_test/test_db_init_locking.py b/tests-unit/app_test/test_db_init_locking.py index b9cfa8c42..6eaf93f3c 100644 --- a/tests-unit/app_test/test_db_init_locking.py +++ b/tests-unit/app_test/test_db_init_locking.py @@ -97,6 +97,43 @@ def test_held_lock_blocks_before_any_migration_work(stale_db): holder.release() +def test_legacy_database_copy_runs_under_file_lock(tmp_path, monkeypatch): + legacy_db = tmp_path / "legacy" / "comfyui.db" + target_db = tmp_path / "current" / "comfyui.db" + legacy_db.parent.mkdir() + legacy_db.write_bytes(b"legacy database") + copied: list[tuple[str, str]] = [] + real_copy = db_module.shutil.copy + + def _copy_while_locked(source: str, destination: str): + contender = FileLock(str(target_db) + ".lock") + try: + with pytest.raises(Timeout): + contender.acquire(timeout=0) + finally: + if contender.is_locked: + contender.release() + copied.append((source, destination)) + return real_copy(source, destination) + + monkeypatch.setattr(db_module.args, "database_url", None) + monkeypatch.setattr(db_module, "get_db_path", lambda: str(target_db)) + monkeypatch.setattr( + db_module, "get_legacy_default_db_path", lambda: str(legacy_db) + ) + monkeypatch.setattr(db_module, "_migrate_and_bind", lambda *_args: None) + monkeypatch.setattr(db_module.shutil, "copy", _copy_while_locked) + monkeypatch.setattr(db_module, "_db_lock", None) + + try: + db_module._init_file_db(f"sqlite:///{target_db}") + finally: + if db_module._db_lock is not None: + db_module._db_lock.release(force=True) + + assert copied == [(str(legacy_db) + ".bak", str(target_db))] + + def test_setup_database_routes_file_lock_to_lock_guidance(monkeypatch, caplog): monkeypatch.setattr(main, "dependencies_available", lambda: True)