fix(download): extract RAR archives in place, not in the IO threadpool (#1459)

Fixes #1458.

A RAR download fails with `Download failed: TypeError`, and the log says
`child watchers are only available on the default loop`. rarfile
extracts by running `unrar` as a subprocess, and `extract_archive_files`
calls it through `run_blocking_io`, so it runs in gevent's IO
threadpool. gevent only lets a subprocess start on the thread that owns
the default loop. `run_blocking_io` already avoids the threadpool for
`subprocess.run`, but a RAR gets to the subprocess through rarfile, so
nothing catches it.

This extracts a RAR on the calling greenlet, where the patched
subprocess waits for `unrar` without blocking anything else. ZIP is pure
Python and stays in the threadpool. I checked it with a real RAR4 file
on a build of main with the stock image's `unrar`: from the threadpool
it raises the TypeError, in place it extracts all 7 files.

Two tests. One checks that a RAR doesn't go through `run_blocking_io`.
The other runs under real gevent patching with an extractor that starts
a process the way rarfile does. Both fail without the change, and the
full suite passes.

I keyed it on the `.rar` suffix to keep it small. Teaching
`run_blocking_io` about rarfile's subprocess would be the more general
fix, but that's a bigger change to a helper a lot of code goes through,
so I left it for you to say if you want it.

shelfmark/download/postprocess/scan.py

@@ -57,6 +57,13 @@ def _format_not_supported_error(rejected_files: list[Path], task: DownloadTask)
     )
 
 
+# rarfile extracts by running `unrar` as a subprocess. Under gevent a subprocess can only be
+# started on the thread that owns the default loop, so in the IO threadpool it raises
+# "TypeError: child watchers are only available on the default loop" and the download fails. The
+# patched subprocess waits cooperatively, so these archives are extracted in place instead.
+_SUBPROCESS_ARCHIVE_SUFFIXES = frozenset({".rar"})
+
+
 def extract_archive_files(
     archive_path: Path,
     output_dir: Path,
@@ -68,12 +75,17 @@ def extract_archive_files(
     content_type = task.content_type
 
     try:
-        extracted_files, warnings, rejected_files = run_blocking_io(
-            extract_archive,
-            archive_path,
-            output_dir,
-            content_type,
-        )
+        if archive_path.suffix.lower() in _SUBPROCESS_ARCHIVE_SUFFIXES:
+            extracted_files, warnings, rejected_files = extract_archive(
+                archive_path, output_dir, content_type
+            )
+        else:
+            extracted_files, warnings, rejected_files = run_blocking_io(
+                extract_archive,
+                archive_path,
+                output_dir,
+                content_type,
+            )
     except ArchiveExtractionError as exc:
         logger.warning(
             "Task %s: archive extraction failed for %s: %s",

tests/download/test_postprocess_scan_blocking_io.py

@@ -1,7 +1,12 @@
 from __future__ import annotations
 
+import subprocess
+import sys
+import textwrap
 from pathlib import Path
 
+import pytest
+
 from shelfmark.core.models import DownloadTask
 from shelfmark.download.postprocess import scan as scan_mod
 
@@ -66,3 +71,77 @@ def test_extract_archive_files_runs_extract_via_run_blocking_io(tmp_path, monkey
     )
 
     assert extract_called, "Expected extract_archive_files to call extract_archive"
+
+
+def test_extract_archive_files_extracts_a_rar_in_place_not_in_the_io_threadpool(
+    tmp_path, monkeypatch
+) -> None:
+    # rarfile runs unrar as a subprocess, which gevent cannot start from its IO threadpool.
+    extracted = []
+
+    def extract_archive_stub(*args, **kwargs):
+        extracted.append(args[0])
+        return [], [], []
+
+    def run_blocking_io_stub(func, *args, **kwargs):
+        raise AssertionError("a RAR must not be extracted through run_blocking_io")
+
+    monkeypatch.setattr(scan_mod, "extract_archive", extract_archive_stub)
+    monkeypatch.setattr(scan_mod, "run_blocking_io", run_blocking_io_stub)
+
+    task = DownloadTask(task_id="t", source="prowlarr", title="Test", content_type="book")
+    scan_mod.extract_archive_files(
+        archive_path=Path("/tmp/fake.RAR"),
+        output_dir=tmp_path,
+        task=task,
+        cleanup_archive=False,
+    )
+
+    assert extracted == [Path("/tmp/fake.RAR")]
+
+
+def test_a_rar_extraction_that_starts_a_process_works_under_gevent(tmp_path) -> None:
+    pytest.importorskip("gevent")
+
+    repo_root = Path(__file__).resolve().parents[2]
+    script = textwrap.dedent(
+        """
+        import os
+        import subprocess
+        import sys
+        import tempfile
+        from pathlib import Path
+
+        from gevent import monkey
+
+        monkey.patch_all()
+        os.environ.setdefault("LOG_ROOT", tempfile.mkdtemp(prefix="shelfmark-log-root-"))
+
+        from shelfmark.core.models import DownloadTask
+        from shelfmark.download.postprocess import scan
+
+        def start_a_process_like_rarfile(*args, **kwargs):
+            subprocess.run([sys.executable, "-c", "pass"], check=True)
+            return [], [], []
+
+        scan.extract_archive = start_a_process_like_rarfile
+        task = DownloadTask(task_id="t", source="prowlarr", title="T", content_type="book")
+        scan.extract_archive_files(
+            archive_path=Path("book.rar"),
+            output_dir=Path(sys.argv[1]),
+            task=task,
+            cleanup_archive=False,
+        )
+        """
+    )
+
+    completed = subprocess.run(
+        [sys.executable, "-c", script, str(tmp_path)],
+        cwd=str(repo_root),
+        capture_output=True,
+        text=True,
+    )
+
+    assert completed.returncode == 0, (
+        f"exit code {completed.returncode}\nstdout:\n{completed.stdout}\nstderr:\n{completed.stderr}"
+    )