diff --git a/TESTING.md b/TESTING.md index b72e858e..fcd0da6a 100644 --- a/TESTING.md +++ b/TESTING.md @@ -2275,7 +2275,8 @@ workspace never asks; previews are a cache the server keeps: `exclusive` job runs (`_auto_thumbnail_blocker`), checked when a write starts (after its slot) and again just before its rename. - Shutdown is bounded and logged: the worker stops at a file boundary, ffmpeg is - killed, the view pool shuts down, and after the app's cleanup a late monitor event + killed, an idle worker leaves (the next output starts a fresh one), the view pool + shuts down, and after the app's cleanup a late monitor event starts nothing. A package reload stops the workers the previous import left. - Cards and strips load previews only: Images in every layout and size, Library cards, Needs review cards and the inspector's cover (a video's cover is its preview frame). diff --git a/py/api/thumbnail_auto.py b/py/api/thumbnail_auto.py index 5ae89148..6e7871ff 100644 --- a/py/api/thumbnail_auto.py +++ b/py/api/thumbnail_auto.py @@ -290,6 +290,9 @@ class AutoThumbnailer: self._queued = set() self._processes = [] self._worker = None + # True from a worker's start until it leaves (under the lock), so an + # idle worker can exit and the next enqueue starts a fresh one. + self._worker_running = False self._closed = False self._busy = False self._batch = 0 @@ -396,9 +399,8 @@ class AutoThumbnailer: self._queued.add(key) self._queue.append((key, Path(src), Path(dst), bool(is_video))) self._busy = True - if self._start_worker and ( - self._worker is None or not self._worker.is_alive() - ): + if self._start_worker and not self._worker_running: + self._worker_running = True worker = threading.Thread( target=self._run, name=WORKER_NAME, daemon=True ) @@ -412,12 +414,10 @@ class AutoThumbnailer: def _run(self): while True: with self._changed: - while not self._queue and not self._closed: - self._busy = False - self._changed.notify_all() - self._changed.wait() - if self._closed: + if self._closed or not self._queue: + # Idle or closed: leave, so no thread outlives its work. self._busy = False + self._worker_running = False self._changed.notify_all() return take = min(len(self._queue), max(1, int(self._workers() or 1))) diff --git a/tests/test_thumbnail_auto.py b/tests/test_thumbnail_auto.py index c3c1674d..8f2cc23f 100644 --- a/tests/test_thumbnail_auto.py +++ b/tests/test_thumbnail_auto.py @@ -359,7 +359,10 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): with patch.object(auto.os, "scandir", counting): resp = await self.client.get(f"{SERVE}thumbnails/Shot_thumb.png?root=0") self.assertEqual(resp.status, 200) - self.assertEqual(len(listings), 1) + # A case-insensitive filesystem (macOS, Windows) finds it by its exact + # name; a case-sensitive one lists the folder once. + found_by_name = (self.output_dir / "Shot.png").exists() + self.assertEqual(len(listings), 0 if found_by_name else 1) self.assertTrue((self.output_dir / "thumbnails" / "Shot_thumb.png").is_file()) async def test_broken_file_falls_back_to_the_original_and_is_remembered(self): @@ -649,44 +652,59 @@ class TestWriteSafety(AutoThumbnailCase): class TestShutdown(AutoThumbnailCase): - def _worker_threads(self): - return [ - t - for t in threading.enumerate() - if t.name == auto.WORKER_NAME and t.is_alive() - ] - async def test_no_thread_remains_after_cleanup_and_capture_cannot_restart(self): path = make_png(self.output_dir / "a.png", size=(64, 64)) self.assertEqual(self.api.queue_capture_thumbnail(str(path)), "queued") + thumbnailer = self.api._auto_thumbnails() + worker = thumbnailer._worker await self._idle() + make_png(self.output_dir / "viewed.png", size=(64, 64)) resp = await self.client.get( - _entry(self.api, self.output_dir, "a.png")["thumbnail_url"] + _entry(self.api, self.output_dir, "viewed.png")["thumbnail_url"] ) await resp.read() pool_threads = list(self.api._preview_pool_threads()) + self.assertTrue(pool_threads) loop = asyncio.get_running_loop() await loop.run_in_executor( None, lambda: self.api.close_auto_thumbnails(final=True) ) - for thread in pool_threads: + for thread in [worker, *pool_threads]: await loop.run_in_executor(None, thread.join, WAIT) - self.assertEqual(self._worker_threads(), []) - self.assertFalse(any(t.is_alive() for t in pool_threads)) + self.assertFalse(thread.is_alive(), thread.name) other = make_png(self.output_dir / "b.png", size=(64, 64)) self.assertEqual(self.api.queue_capture_thumbnail(str(other)), "skipped") - self.assertEqual(self._worker_threads(), []) + restarted = self.api._auto_thumbnailer + self.assertTrue(restarted is None or restarted._worker is None) - async def test_a_reload_stops_the_old_worker(self): + async def test_an_idle_worker_leaves(self): path = make_png(self.output_dir / "a.png", size=(64, 64)) self.api.queue_capture_thumbnail(str(path)) + worker = self.api._auto_thumbnails()._worker await self._idle() - (old,) = self._worker_threads() + loop = asyncio.get_running_loop() + await loop.run_in_executor(None, worker.join, WAIT) + self.assertFalse(worker.is_alive()) + # The next output starts a fresh worker. + other = make_png(self.output_dir / "b.png", size=(64, 64)) + self.assertEqual(self.api.queue_capture_thumbnail(str(other)), "queued") + await self._idle() + self.assertTrue((self.output_dir / "thumbnails" / "b_thumb.png").is_file()) + + async def test_a_reload_stops_the_old_worker(self): + entered, release, _state = self._gate_writes() + path = make_png(self.output_dir / "a.png", size=(64, 64)) + self.api.queue_capture_thumbnail(str(path)) + await self._wait(entered) + old = self.api._auto_thumbnails()._worker old.pm_preview_token = object() # as the module before a reload left it loop = asyncio.get_running_loop() - await loop.run_in_executor(None, auto._stop_stale_workers) + stopping = loop.run_in_executor(None, auto._stop_stale_workers) + release.set() + await stopping await loop.run_in_executor(None, old.join, WAIT) self.assertFalse(old.is_alive()) + self.assertTrue(self.api._auto_thumbnails().closed) async def test_close_kills_ffmpeg_and_logs_a_join_that_timed_out(self): from unittest.mock import MagicMock