From 3a6cf1fa3ad8e6cb56863a2d1cbe48feecb8090a Mon Sep 17 00:00:00 2001 From: lrnselfreliance <33263708+lrnselfreliance@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:31:18 -0600 Subject: [PATCH] files: pass find -printf \0 as an escape, not a Python NUL (#716) * files: pass find -printf \\0 as an escape, not a Python NUL '%h\\0%f\\0%T@\\0' in a Python string is an embedded null byte. subprocess cannot pass that as an argument, so every GNU-find refresh on Linux CI raised ValueError. find wants the two-character escape \\0 in the format string and emits NULs itself. * files: serialize stem backfill so a cancelled compare cannot lock SQLite compare_file_groups runs ensure_file_group_stems in a thread. Cancelling the task does not stop that thread, and a second compare then writes wrolpi_kv on the same connection (tests share one session). A lock plus a version re-check makes the second call a no-op. --- wrolpi/files/lib.py | 38 +++++++++++++++++++------------- wrolpi/files/test/test_worker.py | 21 ++++++++++++++++++ wrolpi/files/worker.py | 7 +++++- 3 files changed, 50 insertions(+), 16 deletions(-) diff --git a/wrolpi/files/lib.py b/wrolpi/files/lib.py index e4e7d650..bfe815b5 100644 --- a/wrolpi/files/lib.py +++ b/wrolpi/files/lib.py @@ -11,6 +11,7 @@ import re import shutil import subprocess import tarfile +import threading import time import urllib.parse import zipfile @@ -493,6 +494,9 @@ _HARDCODED_SUFFIXES = frozenset( # before any compare when this version does not match the value in wrolpi_kv. STEM_ALGORITHM_VERSION = 1 STEM_ALGORITHM_VERSION_KEY = 'stem_algorithm_version' +# Cancelled compare_file_groups leaves ensure_file_group_stems running in a thread. +# Tests (and overlapping refreshes) share one SQLite writer; serialize the backfill. +_stem_backfill_lock = threading.Lock() PART_PARSER = re.compile(r'(.+?)(\.f[\d]{2,3})?(\.info)?(\.\w{3,4})(\.part)', re.IGNORECASE) @@ -850,22 +854,26 @@ def ensure_file_group_stems(): After a successful backfill the kv version matches, and later refreshes skip the table scan. Upsert, from_paths, move, and reorganize must keep stem current. + + A lock serializes callers: cancelling compare_file_groups does not stop the executor + thread, and a second compare would otherwise write wrolpi_kv concurrently (SQLITE_BUSY). """ - stored = get_stem_algorithm_version() - if stored == STEM_ALGORITHM_VERSION: - return - with get_db_curs() as curs: - curs.execute('SELECT id, primary_path FROM file_group') - rows = list(curs.fetchall()) - if rows: - updates = [] - for row in rows: - stem, _ = split_path_stem_and_suffix(row['primary_path']) - updates.append((stem, row['id'])) - with get_db_curs(commit=True) as curs: - for chunk in chunks(updates, 500): - curs.executemany('UPDATE file_group SET stem = ? WHERE id = ?', chunk) - set_stem_algorithm_version(STEM_ALGORITHM_VERSION) + with _stem_backfill_lock: + stored = get_stem_algorithm_version() + if stored == STEM_ALGORITHM_VERSION: + return + with get_db_curs() as curs: + curs.execute('SELECT id, primary_path FROM file_group') + rows = list(curs.fetchall()) + if rows: + updates = [] + for row in rows: + stem, _ = split_path_stem_and_suffix(row['primary_path']) + updates.append((stem, row['id'])) + with get_db_curs(commit=True) as curs: + for chunk in chunks(updates, 500): + curs.executemany('UPDATE file_group SET stem = ? WHERE id = ?', chunk) + set_stem_algorithm_version(STEM_ALGORITHM_VERSION) async def apply_indexers(progress_callback: Callable[[int, int], None] = None): diff --git a/wrolpi/files/test/test_worker.py b/wrolpi/files/test/test_worker.py index 3e32d6bd..5d7cdb9d 100644 --- a/wrolpi/files/test/test_worker.py +++ b/wrolpi/files/test/test_worker.py @@ -1,5 +1,6 @@ """Tests for the file comparison worker.""" import asyncio +import os from unittest import mock import pytest @@ -7,6 +8,7 @@ from sqlalchemy import text from wrolpi.conftest import await_switches from wrolpi.dates import from_timestamp +from wrolpi.vars import IS_MACOS from wrolpi.files.models import FileGroup from wrolpi.files.worker import ( compare_file_groups, @@ -20,6 +22,8 @@ from wrolpi.files.worker import ( _split_nul_field_triples, _entry_from_printf_fields, _normalize_roots, + GNU_FIND_PRINTF, + _stream_gnu_find_entries, ) @@ -1803,6 +1807,23 @@ async def test_refresh_two_directories_does_not_delete_third( 'refreshing A and B deleted a stale FileGroup in C' +def test_gnu_find_printf_is_safe_subprocess_arg(): + """find -printf format must not contain a real NUL; that raises ValueError on exec.""" + assert '\x00' not in GNU_FIND_PRINTF + os.fsencode(GNU_FIND_PRINTF) + + +@pytest.mark.skipif(IS_MACOS, reason='GNU find -printf') +@pytest.mark.asyncio +async def test_gnu_find_printf_emits_nul_records(test_directory, make_files_structure): + """Linux/CI: GNU find -printf with \\0 escapes must actually stream files.""" + make_files_structure(['docs/hello.txt']) + find_args = [str(test_directory), '-type', 'f', '-not', '-path', '*/.*'] + entries = [e async for e in _stream_gnu_find_entries(find_args)] + names = {filename for _directory, filename, _stem, _mtime in entries} + assert 'hello.txt' in names + + def test_split_nul_field_triples_allows_tab_and_newline_in_filename(): """GNU find -printf uses NUL delimiters so a tab/newline in the name is not a field break.""" data = ( diff --git a/wrolpi/files/worker.py b/wrolpi/files/worker.py index 9203bb68..764ed0dd 100644 --- a/wrolpi/files/worker.py +++ b/wrolpi/files/worker.py @@ -51,6 +51,10 @@ QUEUE_STALL_SECONDS = 30 # 0.5s accommodates SD card / FAT32 / exFAT timestamp granularity on Pi 4. MTIME_TOLERANCE_SECONDS = 0.5 +# GNU find -printf format. `\0` here is the two-character find escape, NOT a Python +# NUL: subprocess arguments cannot contain embedded null bytes. +GNU_FIND_PRINTF = r'%h\0%f\0%T@\0' + __all__ = [ 'FileGroupDiff', 'FileComparisonResult', @@ -59,6 +63,7 @@ __all__ = [ 'compare_file_groups', 'count_files', 'file_worker', + 'GNU_FIND_PRINTF', ] @@ -420,7 +425,7 @@ def _entry_from_printf_fields(directory_b: bytes, filename_b: bytes, mtime_b: by async def _stream_gnu_find_entries(find_args: list[str]) -> AsyncGenerator[Tuple[str, str, str, float], None]: """Stream (directory, filename, stem, mtime) using GNU find -printf with NUL delimiters.""" proc = await asyncio.create_subprocess_exec( - 'find', *find_args, '-printf', '%h\0%f\0%T@\0', + 'find', *find_args, '-printf', GNU_FIND_PRINTF, stdout=asyncio.subprocess.PIPE, ) buf = b''