Files
ComfyAssets-ComfyUI_PromptM…/tests/test_library_move_review.py
T
Vito b1e7abd380 fix(threads): stop and join the prompt cleanup thread, the image worker and the observer (#302, #370) (#372)
* fix(threads): stop and join the prompt cleanup thread, the image worker and the observer (#302, #370)

- PromptTracker: the cleanup thread waits on a stop event instead of
  time.sleep; new shutdown(timeout=2) joins it and returns whether it ended.
- ImageGenerationHandler.shutdown(timeout=5) now joins the worker and
  returns a bool; new close() is final: schedule() refuses and a sealed
  run's history re-check is dropped, so no worker restarts afterwards.
- ImageMonitor.stop_monitoring(timeout=5) closes the handler first, then
  stops and joins the watchdog observer within one deadline.
- Every test that builds a real tracker, handler or monitor ends with its
  final stop and an assertion, before its folder and databases go.
- #370: the wrapper/session history test drains before dispose(); the
  held-write bound test uses 30 s ceilings instead of tight wall clocks.

* test(history): drain the writer before asserting a bounded close it does not test (#370)

* fix(threads): a later stop waits again for a worker or observer an earlier stop left running (#302)

Review findings on #372:
- ImageGenerationHandler keeps the workers a stop signalled that had not
  ended; every later shutdown/close joins them again, so a second call no
  longer returns True while the first worker still runs.
- ImageMonitor.stop_monitoring keeps an observer or handler it left
  running and stops and joins it again on the next call.
- A closed handler takes no awaiting history path, and a re-check that
  raced close() is not kept due.
- Tests: second stops of a busy worker and a live observer, the closed
  handler's re-check paths, the handle test asserts the worker ended and
  stops before it asserts, and a no-new-thread check instead of set
  equality.

* test(browser): the System reset restores every settings value, safety_backup included

The frontend coverage shard of #372 failed in workspace-system-tagging:
the review read "No safety backup (the policy is off)". The backups spec
saves safety_backup=false, and /__test__/system/reset restored config.json
but not PromptManagerConfig.SAFETY_BACKUP (nor INFINITE_SCROLL), so a
later spec on the same worker server saw the policy off. The reset now
captures and restores every non-tagging value POST /settings can change.
2026-10-27 02:15:23 -07:00

462 lines
18 KiB
Python

"""Move library, final review round (#249 S6b PR #318).
- I1: a switch that fails at any point is undone while both files are still
locked: the copy is removed, the old library stays in use and a restart
keeps it. A follow-up after a complete switch only logs.
- I2: a background write (run counting, the image monitor) that fails
because Move library switched runs once more, on the new library; a write
racing the switch on a real connection (a run count, an image link) lands
in the new library only.
- The switching thread never closes a connection another thread is inside
a statement on (a use-after-free in SQLite: "Windows fatal exception:
access violation"); it interrupts it and the owner closes it.
- M2: the copy is verified through a file URI that survives ``#`` and ``%``.
- M6: a restart after a crash between the switch and the rename renames the
old files, never deletes them.
"""
import contextlib
import json
import os
import sqlite3
import sys
import threading
import unittest
from unittest import mock
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
from database import library_move as lm # noqa: E402
from database import models # noqa: E402
from database.operations import PromptDatabase # noqa: E402
from tests.background_stops import close_handler # noqa: E402
from tests.open_handles import assert_closed # noqa: E402
from utils import library_location as ll # noqa: E402
from utils import usage_tracking # noqa: E402
from utils.hashing import generate_prompt_hash # noqa: E402
try:
from tests.test_library_move import KEY, SimulatedCrash, _MoveCase, _rows
except ImportError: # discovered with tests/ as the top-level directory
from test_library_move import KEY, SimulatedCrash, _MoveCase, _rows
WAIT = 30
def _run_count(path, text):
uri = "file:" + path.replace(os.sep, "/") + "?mode=ro"
with contextlib.closing(sqlite3.connect(uri, uri=True)) as conn:
row = conn.execute("SELECT run_count FROM prompts WHERE text=?", (text,))
return row.fetchone()[0]
class _CrossThreadCloseProbe:
"""Record every ``close()`` this thread calls on a connection in use elsewhere.
The writer marks its connection with ``using`` when its statement starts.
Closing that connection from the switching thread while the writer is
inside the statement frees memory SQLite is still using; on Windows the
process dies with an access violation, on Linux the writer sees garbage.
"""
def __init__(self):
self.in_use = set()
self.violations = []
def using(self, conn):
self.in_use.add(conn)
def _profile(self, _frame, event, arg):
if event != "c_call" or getattr(arg, "__name__", "") != "close":
return
target = getattr(arg, "__self__", None)
if isinstance(target, sqlite3.Connection) and target in self.in_use:
self.violations.append(threading.current_thread().name)
@contextlib.contextmanager
def watching(self):
previous = sys.getprofile()
sys.setprofile(self._profile)
try:
yield self
finally:
sys.setprofile(previous)
class TestFailedSwitchIsUndone(_MoveCase):
"""I1: whatever part of the switch fails, nothing was moved."""
def _assert_not_moved(self, caught):
self.assertEqual(caught.exception.code, "switch-failed")
self.assertIn("Nothing was moved", caught.exception.message)
self.assertIsNone(ll.current_override())
self.assertEqual(self.db.model.db_path, self.old_db)
# Writes keep landing in the old library, and a restart keeps it.
self.db.save_prompt("written after the failed switch")
self.assertIn("written after the failed switch", repr(_rows(self.old_db)))
self.assertEqual(self.location().source, ll.SOURCE_NODE_FOLDER)
self.assertNotEqual(ll.read_marker(self.target).get("state"), "moved")
self.assert_target_clean()
self.assertTrue(os.path.isfile(self.config))
assert_closed(self, self.target)
def _failing(self, point):
real_switch_to = ll.switch_to
real_repoint = models.PromptModel.repoint_instances
def switch(old_db, location, detached):
if point == "before":
raise RuntimeError("switch failed before anything")
if point == "resolver":
real_switch_to(location)
raise RuntimeError("switch failed after the resolver")
if point == "repoint":
real_switch_to(location)
detached.extend(real_repoint(old_db, location.db_path))
raise RuntimeError("switch failed after the models")
lm.switch_process(old_db, location, detached)
raise RuntimeError("switch failed at its very end")
return switch
def test_a_switch_failing_at_any_point_is_undone(self):
node = PromptDatabase(self.old_db)
node.get_prompt_by_id(1)
for point in ("before", "resolver", "repoint", "complete"):
with self.subTest(point=point):
with self.assertRaises(lm.MoveFailed) as caught:
self.move(switch=self._failing(point))
self._assert_not_moved(caught)
self.assertEqual(node.model.db_path, self.old_db)
self.assertEqual(node.get_prompt_by_id(1)["text"], "a cat number 0")
def test_a_failed_switch_closes_the_old_connections_it_detached(self):
node = PromptDatabase(self.old_db)
for point in ("repoint", "complete"):
with self.subTest(point=point):
before = node.model.get_connection()
with self.assertRaises(lm.MoveFailed):
self.move(switch=self._failing(point))
with self.assertRaises(sqlite3.ProgrammingError):
before.execute("SELECT 1")
# The model reopens the old library on next use.
self.assertEqual(node.get_prompt_by_id(1)["text"], "a cat number 0")
def test_a_failing_follow_up_after_the_switch_only_logs(self):
def follow_up():
raise RuntimeError("the health report could not be dropped")
with self.assertLogs("prompt_manager.database.library_move", "ERROR"):
result = self.move(after_switch=follow_up)
self.assertIsNone(result.retire_problem)
self.assertEqual(self.db.model.db_path, self.new_db)
self.assertTrue(
os.path.isfile(
os.path.join(self.ext, "prompts.db.moved-to-__promptmanager")
)
)
class _Flaky:
"""A write that fails once as if Move library switched while it waited."""
def __init__(self, db, name):
self.db, self.name, self.calls = db, name, 0
self.real = getattr(db, name)
def __call__(self, *args, **kwargs):
self.calls += 1
if self.calls == 1:
self.db.model.close_all() # what the switch does: a new generation
raise sqlite3.ProgrammingError("Cannot operate on a closed database.")
return self.real(*args, **kwargs)
class TestBackgroundWritesSurviveTheSwitch(_MoveCase):
"""I2: run counting and the image monitor write again on a fresh connection."""
def test_run_counting_retries_once_after_a_switch(self):
db = PromptDatabase(self.old_db)
text = "a counted prompt"
db.save_prompt(text, prompt_hash=generate_prompt_hash(text))
flaky = _Flaky(db, "record_prompt_use")
with mock.patch.object(db, "record_prompt_use", flaky):
pending = usage_tracking.PendingFirstUse()
usage_tracking.count_runs([(generate_prompt_hash(text), text)], db, pending)
self.assertEqual(flaky.calls, 2)
self.assertEqual(_run_count(self.old_db, text), 1)
def test_a_failure_without_a_switch_is_not_retried(self):
db = PromptDatabase(self.old_db)
calls = []
def broken():
calls.append(1)
raise sqlite3.OperationalError("disk I/O error")
with self.assertRaises(sqlite3.OperationalError):
db.survive_library_switch(broken)
self.assertEqual(calls, [1])
def test_an_image_link_racing_the_switch_lands_in_the_new_library(self):
from utils.image_monitor import ImageGenerationHandler
self.db.model.close_all()
image = os.path.join(self.comfy, "output", "late.png")
writer_db = PromptDatabase(self.old_db)
handler = ImageGenerationHandler(writer_db, mock.MagicMock())
self.addCleanup(close_handler, self, handler)
started = threading.Event()
probe = _CrossThreadCloseProbe()
outcome = {}
def write():
conn = writer_db.model.get_connection()
def trace(sql):
if sql.lstrip().upper().startswith(("BEGIN", "INSERT")):
probe.using(conn)
started.set()
conn.set_trace_callback(trace)
try:
# No metadata: the fallback link, not the resolved pair.
handler.link_image_to_prompt(image, {"id": 1}, {})
outcome["ok"] = True
except Exception as error: # pragma: no cover - reported below
outcome["error"] = error
finally:
writer_db.model.close()
thread = threading.Thread(target=write)
def checkpoint(step):
if step == "locked":
# The old file is write-locked now: start the writer and
# wait until its first write is running (it then waits on us).
thread.start()
self.assertTrue(started.wait(WAIT))
with probe.watching():
result = self.move(checkpoint=checkpoint)
thread.join(WAIT)
self.assertFalse(thread.is_alive())
self.assertEqual(probe.violations, [])
self.assertEqual(outcome, {"ok": True})
self.assertIsNone(result.retire_problem)
moved = [
row
for row in _rows(self.new_db)["generated_images"]
if "late.png" in repr(row)
]
self.assertEqual(len(moved), 1)
renamed = os.path.join(self.ext, "prompts.db.moved-to-__promptmanager")
self.assertNotIn("late.png", repr(_rows(renamed).get("generated_images")))
def test_an_image_that_was_not_linked_is_not_reported_as_linked(self):
from utils.image_monitor import ImageGenerationHandler
db = PromptDatabase(self.old_db)
handler = ImageGenerationHandler(db, mock.MagicMock())
self.addCleanup(close_handler, self, handler)
image = os.path.join(self.comfy, "output", "orphan.png")
with mock.patch.object(handler, "logger") as log:
handler.link_image_to_prompt(image, {"id": 99999}, {})
messages = [str(call) for call in log.debug.call_args_list]
self.assertFalse([m for m in messages if "Successfully linked" in m])
self.assertNotIn("orphan.png", repr(_rows(self.old_db)["generated_images"]))
def test_a_write_racing_the_switch_lands_in_the_new_library(self):
text = "a racing prompt"
self.db.save_prompt(text, prompt_hash=generate_prompt_hash(text))
self.db.model.close_all()
writer_db = PromptDatabase(self.old_db)
thread, started, probe, outcome = self._racing_run_count(writer_db, text)
def checkpoint(step):
if step == "locked":
# The old file is write-locked now: start the writer and
# wait until its UPDATE is running (it then waits on us).
thread.start()
self.assertTrue(started.wait(WAIT))
with probe.watching():
result = self.move(checkpoint=checkpoint)
# The writer's old connection was closed before the old file was
# renamed: Windows refuses to rename an open file.
self.assertIsNone(result.retire_problem)
thread.join(WAIT)
self.assertFalse(thread.is_alive())
self.assertEqual(probe.violations, [])
self.assertEqual(outcome, {"ok": True})
self.assertEqual(_run_count(self.new_db, text), 1)
renamed = os.path.join(self.ext, "prompts.db.moved-to-__promptmanager")
self.assertEqual(_run_count(renamed, text), 0)
assert_closed(self, renamed)
def test_a_write_racing_a_failed_switch_lands_in_the_old_library(self):
text = "a prompt racing a failed switch"
self.db.save_prompt(text, prompt_hash=generate_prompt_hash(text))
self.db.model.close_all()
writer_db = PromptDatabase(self.old_db)
thread, started, probe, outcome = self._racing_run_count(writer_db, text)
def checkpoint(step):
if step == "locked":
thread.start()
self.assertTrue(started.wait(WAIT))
def switch(old_db, location, detached):
lm.switch_process(old_db, location, detached)
raise RuntimeError("switch failed at its very end")
with probe.watching():
with self.assertRaises(lm.MoveFailed):
self.move(checkpoint=checkpoint, switch=switch)
# The copy was removed: no connection was left open on it.
self.assert_target_clean()
assert_closed(self, self.target)
thread.join(WAIT)
self.assertFalse(thread.is_alive())
self.assertEqual(probe.violations, [])
self.assertEqual(outcome, {"ok": True})
self.assertEqual(_run_count(self.old_db, text), 1)
def test_a_retire_failure_after_a_complete_switch_does_not_fail_the_move(self):
node = PromptDatabase(self.old_db)
node.get_prompt_by_id(1)
real = models.PromptModel.retire_detached
calls = []
def failing(connections, logger=None):
calls.append(len(connections))
real(connections, logger) # closed, but the call still fails
raise RuntimeError("retiring failed")
with mock.patch.object(
models.PromptModel, "retire_detached", staticmethod(failing)
):
with self.assertLogs("prompt_manager.database.library_move", "ERROR"):
result = self.move()
self.assertEqual(len(calls), 1)
self.assertGreater(calls[0], 0)
self.assertEqual(node.model.db_path, self.new_db)
self.assertEqual(result.location.db_path, self.new_db)
self.assertTrue(
os.path.isfile(
os.path.join(self.ext, "prompts.db.moved-to-__promptmanager")
)
)
@staticmethod
def _racing_run_count(writer_db, text):
"""A run count whose UPDATE marks its connection in use for the probe."""
started = threading.Event()
probe = _CrossThreadCloseProbe()
outcome = {}
def write():
conn = writer_db.model.get_connection()
def trace(sql):
if sql.startswith("UPDATE"):
probe.using(conn)
started.set()
conn.set_trace_callback(trace)
try:
usage_tracking.count_runs(
[(generate_prompt_hash(text), text)],
writer_db,
usage_tracking.PendingFirstUse(),
)
outcome["ok"] = True
except Exception as error: # pragma: no cover - reported below
outcome["error"] = error
finally:
writer_db.model.close()
return threading.Thread(target=write), started, probe, outcome
class TestVerifyUri(unittest.TestCase):
"""M2: a copy whose path holds ``#`` and ``%`` verifies."""
def test_copy_path_with_uri_characters_verifies(self):
import tempfile
with tempfile.TemporaryDirectory() as tmp:
folder = os.path.join(tmp, "lib #1 %20")
os.makedirs(folder)
path = os.path.join(folder, "copy.db")
db = PromptDatabase(path)
db.save_prompt("a prompt")
db.model.close_all()
with contextlib.closing(sqlite3.connect(path)) as conn:
conn.execute("PRAGMA journal_mode=DELETE")
counts, documents = lm._describe(conn)
lm._verify_copy(path, counts, documents)
models.PromptModel.close_all_instances()
class TestRestartFinishesTheRename(_MoveCase):
"""M6: the marker names the old files; a restart renames them."""
def _crash_after_switch(self):
def checkpoint(step):
if step == "switched":
raise SimulatedCrash(step)
with self.assertRaises(SimulatedCrash):
self.move(checkpoint=checkpoint)
models.PromptModel.close_all_instances()
ll.clear_override()
def test_a_restart_renames_the_old_files_never_deletes(self):
self._crash_after_switch()
self.assertTrue(os.path.isfile(self.old_db))
restarted = self.location()
self.assertEqual(restarted.db_path, self.new_db)
marker = ll.read_marker(self.target)
self.assertEqual(
(marker["from"], marker["config_from"]), (self.old_db, self.config)
)
renamed = lm.finish_interrupted_move(restarted)
self.assertEqual(
sorted(renamed),
[
"config.json.moved-to-__promptmanager",
"prompts.db.moved-to-__promptmanager",
],
)
self.assertFalse(os.path.exists(self.old_db))
moved_config = os.path.join(self.ext, "config.json.moved-to-__promptmanager")
with open(moved_config, encoding="utf-8") as handle:
self.assertEqual(
json.load(handle)["integrations"]["lora_manager"]["civitai_api_key"],
KEY,
)
self.assertEqual(lm.finish_interrupted_move(self.location()), [])
def test_nothing_is_renamed_while_the_old_file_is_in_use(self):
self._crash_after_switch()
reader = sqlite3.connect(self.old_db)
try:
reader.execute("BEGIN")
reader.execute("SELECT COUNT(*) FROM prompts").fetchone()
PromptDatabase(self.old_db).save_prompt("in the wal")
self.assertEqual(lm.finish_interrupted_move(self.location()), [])
self.assertTrue(os.path.isfile(self.old_db))
finally:
reader.rollback()
reader.close()
models.PromptModel.close_all_instances()
def test_a_node_folder_library_has_nothing_to_finish(self):
self.assertEqual(lm.finish_interrupted_move(self.location()), [])
self.assertTrue(os.path.isfile(self.old_db))
if __name__ == "__main__":
unittest.main()