From 9f9d7414f6f0e2e2e202610dc71acab7de3e2c63 Mon Sep 17 00:00:00 2001 From: dantidote Date: Fri, 15 May 2026 08:32:33 -0400 Subject: [PATCH] raop: fix read-side deadlock in PatchedIceCastClient (#2849) When miniaudio.stream_any's decoder init issues a read larger than (BUFFER_SIZE - position) while position is still inside the seekable headroom, PatchedIceCastClient.read deadlocks against its downloader thread: - read() polls until len(buffer) >= num_bytes, where len(buffer) is size (post-position bytes). - The downloader's _buffer.fits(BLOCK_SIZE) guard checks the *raw* buffer length, not size, so once raw is at BUFFER_SIZE it refuses to add even though size has room to grow with consumption. - Reader waits for the buffer to grow; downloader cannot grow it until the reader consumes. Polling loop hits DEFAULT_TIMEOUT (10s) and miniaudio surfaces the resulting OperationTimeoutError as DecodeError('failed to init decoder', -1). Small streams escape because the HTTP body completes within the 10s window: _stop_stream flips True and the loop's other exit fires. Detect 'raw buffer cannot grow' in the polling loop and let self._buffer.get() return a short read. miniaudio's drmp3 handles short reads by re-calling; once subsequent reads advance position past HEADROOM_SIZE the buffer releases its headroom and becomes a normal sliding window, after which the downloader can refill. Tests use pytest-httpserver to serve the existing static_3sec.ogg fixture (small, exercises the fast path) and a new audio_long.mp3 (~80 KiB silence, just over BUFFER_SIZE) to trigger the deadlock. On master the long test fails after ~10s with DecodeError(-1); with this fix both tests pass in under a second. Also bump the miniaudio dev pin from 1.61 to 1.71 in requirements/requirements.txt. The deadlock fires on 1.71's decoder init call sequence (the new seek(0, END) plus extra round-trip leaves position at 2048 when the read(65536) fires); 1.61's init ends with seek(0, START) which resets position before the same read, hiding the bug. Real users `pip install pyatv` and resolve miniaudio>=1.45 to the newest available (1.71 today, and only 1.71 has Python 3.14 wheels), but the old dev pin meant pyatv's own CI didn't exercise the bug pattern users hit. Bumping aligns CI with what users actually run so the new regression test serves as a real guard. --- pyatv/protocols/raop/audio_source.py | 2 + requirements/requirements.txt | 2 +- tests/data/audio_long.mp3 | Bin 0 -> 81128 bytes tests/protocols/raop/test_audio_source.py | 50 ++++++++++++++++++++++ 4 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tests/data/audio_long.mp3 create mode 100644 tests/protocols/raop/test_audio_source.py diff --git a/pyatv/protocols/raop/audio_source.py b/pyatv/protocols/raop/audio_source.py index ea97859e5..729cbd842 100644 --- a/pyatv/protocols/raop/audio_source.py +++ b/pyatv/protocols/raop/audio_source.py @@ -485,6 +485,8 @@ def read(self, num_bytes: int) -> bytes: # TODO: Should not be based on polling while len(self._buffer) < num_bytes and not self._stop_stream: + if not self._buffer.fits(1): + break if time.monotonic() - start_time > DEFAULT_TIMEOUT: raise OperationTimeoutError("timed out reading from stream") diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 9b6594d3e..b748a6e1e 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -4,7 +4,7 @@ cryptography==46.0.3 chacha20poly1305-reuseable==0.13.2 ifaddr==0.2.0 ifaddr==0.2.0 -miniaudio==1.61 +miniaudio==1.71 protobuf==6.33.2 pydantic==2.12.5 requests==2.32.5 diff --git a/tests/data/audio_long.mp3 b/tests/data/audio_long.mp3 new file mode 100644 index 0000000000000000000000000000000000000000..8092e5d2314c2ac4d200dae50dc45672030357f9 GIT binary patch literal 81128 zcmeI)Z^WKu0EY2JFE;IslLNJgXC!`ZV zfrMZnJx)j`fC34@Kzf{zP5=cGf`RlnA)NpUBm@KLaY8x)6i5gL(&L150w|CW45Y^i z=>$+9As9%H6VeHwKteE(9w(#|K!Jo{AU#e2X3j0Tf6G2GZk%bOI=l z5DcWp3F!n-AR!n?j}y`fpg=+}kRB(b6F`B4U?4qCNGE^-3Bf>moRCfc1rmaR^f)1% zV0i%@s0)qDfx6HL`}nc`40Ra`J_HHDKzf{zP5=cGf`RlnA)NpUBm@KLaY8x)6i5gL z(&L150w|CW45Y^i=>$+9As9%H6VeHwKteE(9w(#|K!Jo{AU#e00k0)f%G^bod60X1Ow@DLOKByNC*bf zMvr@j5`qQ#DF*}f48cHpoRCfc1rmaR^f)1%016}o1L<)>Isp_&2nN#QgmeNZkPr-{ z#|h~KP#_@~NRJcJ37|kiFpwT6q!U1agkT^&PDm$!0tvxDdYq6>00k0)f%G^bonUzZ z9jFV9%Ypj+f3S}q>(5Y^vEW0H5DcWp3F!n-AR!n?j}y`fpg=+}kRB(b6F`B4U?4qC zNGE^-3Bf>moRCfc1rmaR^f)1%016}o1L<)>Isp_&2nN#QgmeNZkPr-{#|h~KqZjxS Dx4Tb8 literal 0 HcmV?d00001 diff --git a/tests/protocols/raop/test_audio_source.py b/tests/protocols/raop/test_audio_source.py new file mode 100644 index 000000000..42f7a9edd --- /dev/null +++ b/tests/protocols/raop/test_audio_source.py @@ -0,0 +1,50 @@ +"""Tests for pyatv.protocols.raop.audio_source. + +Includes a regression test for #2849: streaming a media file larger than +PatchedIceCastClient's internal BUFFER_SIZE must not deadlock during +miniaudio's decoder init. +""" + +import pytest +from pytest_httpserver import HTTPServer + +from pyatv.protocols.raop.audio_source import ( + BUFFER_SIZE, + InternetSource, +) + +from tests.utils import data_path + +pytestmark = pytest.mark.asyncio + +SHORT_FIXTURE = "static_3sec.ogg" # existing, ~4 KiB +LONG_FIXTURE = "audio_long.mp3" # new, just over BUFFER_SIZE + + +async def _serve_and_open(httpserver: HTTPServer, body: bytes, name: str) -> None: + httpserver.expect_request("/" + name).respond_with_data( + body, content_type="application/octet-stream" + ) + url = httpserver.url_for("/" + name) + src = await InternetSource.open(url, sample_rate=44100, channels=2, sample_size=2) + try: + frames = await src.readframes(352) + assert frames, "InternetSource returned no audio frames after init" + finally: + await src.close() + + +async def test_internet_source_short_stream(httpserver: HTTPServer): + """Streaming a small file over HTTP works (fast path, no deadlock window).""" + with open(data_path(SHORT_FIXTURE), "rb") as fh: + body = fh.read() + assert len(body) < BUFFER_SIZE + await _serve_and_open(httpserver, body, SHORT_FIXTURE) + + +async def test_internet_source_long_stream_no_deadlock(httpserver: HTTPServer): + """Regression test for #2849: streaming a file > BUFFER_SIZE must not deadlock.""" + with open(data_path(LONG_FIXTURE), "rb") as fh: + body = fh.read() + assert len(body) > BUFFER_SIZE + await _serve_and_open(httpserver, body, LONG_FIXTURE)