From 327e1eb4b81559f63fae1ca87e0c76d767cca1c8 Mon Sep 17 00:00:00 2001 From: Alex Shnitman Date: Fri, 21 Aug 2026 09:23:09 +0200 Subject: [PATCH] fix: stop stating the output file on every progress tick (#980) yt-dlp documents 'filename' as always present in a progress hook, so the update_status branch that stats it ran on every forwarded tick: throttled to one every 0.5s per download, times MAX_CONCURRENT_DOWNLOADS. Those are blocking syscalls on the event loop, and when the filesystem is slow each one freezes every other request the server is serving -- which is what a bare GET timing out at >10s looks like from outside. The call was also useless while it was expensive. Until the download finishes the bytes live in tmpfilename; 'filename' is the destination, which does not exist yet, so os.path.exists() returned False and size stayed None. It only yields a real value on a terminal status, and the Downloading table has no size column, so nothing displayed it before completion either way. Stat only when the status is 'finished'. That covers both moments a file genuinely exists at that path: yt-dlp's own finished status, and the MoveFiles postprocessor reporting the final merged name. Co-Authored-By: Claude Opus 5 --- app/tests/test_ytdl_utils.py | 63 +++++++++++++++++++++++++++++++++++- app/ytdl.py | 12 ++++++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/app/tests/test_ytdl_utils.py b/app/tests/test_ytdl_utils.py index 434a7ef..c5363f4 100644 --- a/app/tests/test_ytdl_utils.py +++ b/app/tests/test_ytdl_utils.py @@ -2,6 +2,7 @@ from __future__ import annotations +import asyncio import os import pickle import signal @@ -11,7 +12,8 @@ import threading import types import unittest from pathlib import Path -from unittest.mock import MagicMock, patch +from concurrent.futures import ThreadPoolExecutor +from unittest.mock import AsyncMock, MagicMock, patch fake_yt_dlp = types.ModuleType("yt_dlp") fake_networking = types.ModuleType("yt_dlp.networking") @@ -1176,3 +1178,62 @@ class PotProviderUrlsTests(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +class UpdateStatusFileStatTests(unittest.IsolatedAsyncioTestCase): + """The progress path must not touch the filesystem on the event loop. + + yt-dlp reports 'filename' on every progress tick, but until the download + finishes that path does not exist yet -- the bytes are in 'tmpfilename'. + Stating it per tick meant blocking syscalls on the event loop twice a + second per download, always answering None. See issue #980. + """ + + async def _run_update_status(self, statuses): + import queue as _queue + + download = _make_test_download() + download.download_dir = "/tmp" + source = _queue.Queue() + for status in statuses: + source.put(status) + source.put(None) + download.status_queue = source + download.loop = asyncio.get_running_loop() + download._executor = ThreadPoolExecutor(max_workers=1) + notifier = MagicMock() + notifier.updated = AsyncMock() + download.notifier = notifier + + stat_calls = [] + + def record_exists(path): + stat_calls.append(path) + return False + + try: + with patch("ytdl.os.path.exists", side_effect=record_exists): + await download.update_status() + finally: + download._executor.shutdown(wait=True) + return download, stat_calls + + async def test_downloading_ticks_do_not_stat_the_output_file(self): + ticks = [ + {"status": "downloading", "filename": "/tmp/v.mp4", + "tmpfilename": "/tmp/v.mp4.part", "downloaded_bytes": i} + for i in range(1, 6) + ] + download, stat_calls = await self._run_update_status(ticks) + + self.assertEqual(stat_calls, []) + self.assertEqual(download.info.filename, "v.mp4") + + async def test_finished_status_still_stats_the_output_file(self): + download, stat_calls = await self._run_update_status([ + {"status": "downloading", "filename": "/tmp/v.mp4", "downloaded_bytes": 1}, + {"status": "finished", "filename": "/tmp/v.mp4"}, + ]) + + self.assertEqual(stat_calls, ["/tmp/v.mp4"]) + self.assertIsNone(download.info.size) diff --git a/app/ytdl.py b/app/ytdl.py index 21a5182..5d28d3f 100644 --- a/app/ytdl.py +++ b/app/ytdl.py @@ -1027,7 +1027,17 @@ class Download: if not rel_name.lower().endswith(allowed_caption_exts): continue self.info.filename = rel_name - self.info.size = os.path.getsize(fileName) if os.path.exists(fileName) else None + # Stat only on a terminal status. yt-dlp documents 'filename' as + # always present in a progress hook, but until the download + # finishes the bytes are in tmpfilename and 'filename' is a + # destination that does not exist yet -- so this was two + # blocking syscalls on the event loop, twice a second per + # active download, to arrive at None. A stat that takes seconds + # on a contended filesystem stalls every other request the + # server is serving. Nothing displays the size before + # completion: the Downloading table has no size column. + if status.get('status') == 'finished': + self.info.size = os.path.getsize(fileName) if os.path.exists(fileName) else None if getattr(self.info, 'download_type', '') == 'thumbnail': # The thumbnail convertor always emits a .jpg, but yt-dlp may # report the pre-conversion media/thumbnail extension