Keep the RTSPS proxy's handler set off the server object (issue #3001)

asyncio's Server has a __dict__ and uvloop's, a Cython cdef class, does
not, so the attribute added in 1.2.5.4 raised AttributeError under uvloop.
Every RTSP camera failed before opening a socket, which is the
diagnostic's capture_exception at 0 ms.

Our own unit files all pin --loop asyncio for #1896 and were never
affected. The reports come from units we do not write: the Proxmox VE
Helper-Scripts LXC pins no loop, and installs predating that fix never
gained the flag because update.sh does not rewrite unit files. The loop
is not ours to assume, so fix the code rather than add another flag.

The set moves to a module-level WeakKeyDictionary, keyed weakly so an
abandoned proxy retires its own entry rather than leaking one and later
handing a new server a dead one's handlers.

Pinned on a real uvloop loop and, for hosts without uvloop, against a
__slots__ server; conftest builds its loop from the default policy, so
nothing in the suite had ever run the branch that broke.

Also routes the two external-camera teardowns through close_tls_proxy,
which #2968 introduced and left them out of.

-----

Say so at startup when running on uvloop (issue #3001)

An install on the wrong loop had no way to find out it was. #3001 was
loud enough to notice; the #1896 upload truncation it is also exposed to
is silent, and shows up as a print failing from a file that was corrupt
on arrival.

One WARNING in the lifespan naming the loop, the risk and the flag to
add. A warning and not a refusal: uvicorn has already chosen its loop by
the time any application code runs, and a server that answers requests
beats one that will not boot.

Asks the running loop what it is rather than whether uvloop imports --
uvicorn[standard] installs uvloop everywhere, so its presence says
nothing -- and matches on the module name so the question never imports
uvloop on a host without it.

-----

Repair a service file written before the --loop asyncio pin (issue #3001)

install.sh has pinned the loop since #1896, but nothing has ever
rewritten an existing service file, so every native install created
between 2025-11-28 (when uvicorn[standard] brought uvloop into the venv)
and 2026-07-05 still runs on uvloop no matter how often it is updated.

Both update scripts now add the flag themselves while the service is
stopped, so it takes effect on the same restart -- systemd via sed,
launchd via PlistBuddy, each backing the file up first and inserting
nothing but the flag.

Refuses to edit and explains instead when the shape is not a plain
single-line uvicorn unit: a wrapper script, a continued ExecStart,
several of them, a read-only file, or a service with drop-ins, since a
drop-in may be what defines ExecStart and editing the fragment would
change nothing while reporting success. A deliberate --loop uvloop is
left alone. Reads the effective ExecStart from systemd rather than the
file, so it is idempotent.
This commit is contained in:
maziggy
2026-08-30 08:01:42 +02:00
parent 3f1ed85791
commit 0dfcff5925
10 changed files with 449 additions and 27 deletions
+72 -6
View File
@@ -1,10 +1,10 @@
"""Asyncio event-loop exception handlers used at app startup.
"""Event-loop concerns handled at app startup.
Currently houses a single Windows-specific filter for the noisy
``_ProactorBasePipeTransport._call_connection_lost`` ``WinError 10054``
that fires every time a printer / MQTT broker / camera RSTs a TCP socket
instead of closing it cleanly. See ``install_proactor_reset_filter`` for
the why and the failure mode it suppresses.
Two of them, both about which loop implementation Bambuddy finds itself on.
``install_proactor_reset_filter`` silences the noisy Windows Proactor
cleanup-RST that fires whenever a printer / MQTT broker / camera RSTs a socket
instead of closing it; ``warn_if_running_on_uvloop`` says so out loud when the
loop is uvloop, which Bambuddy is not launched on and does not want.
"""
from __future__ import annotations
@@ -71,3 +71,69 @@ def install_proactor_reset_filter(loop: asyncio.AbstractEventLoop | None = None)
loop = asyncio.get_running_loop()
loop.set_exception_handler(_proactor_reset_filter)
return True
# Every launch path Bambuddy ships pins ``--loop asyncio``: the Dockerfile,
# install/install.sh, deploy/bambuddy.service, the Windows service and the
# SpoolBuddy installer. That flag was added for #1896 and is load-bearing --
# see the warning text below for what it holds up.
_LOOP_FLAG = "--loop asyncio"
def running_on_uvloop(loop: asyncio.AbstractEventLoop | None = None) -> bool:
"""Is `loop` (or the running loop) a uvloop loop?
Asks the loop what it is rather than whether uvloop imports: uvloop is a
hard dependency here -- ``requirements.txt`` pins ``uvicorn[standard]``,
which installs it on Linux -- so its mere presence says nothing. Matching
on the module name rather than ``isinstance(loop, uvloop.Loop)`` keeps this
from importing uvloop just to ask the question, which on a host without it
would be an ImportError in the middle of startup.
"""
if loop is None:
try:
loop = asyncio.get_running_loop()
except RuntimeError:
return False
return type(loop).__module__.split(".")[0] == "uvloop"
def warn_if_running_on_uvloop(loop: asyncio.AbstractEventLoop | None = None) -> bool:
"""Log a loud warning when the process is running on uvloop.
Bambuddy is developed, tested and shipped on asyncio's own loop, and two
faults have already been traced to uvloop's differences from it:
* #1896 -- uvloop's SSL layer can drop buffered data when a client closes
without a TLS close_notify, so a Virtual Printer FTP upload can be
truncated, acked ``226``, archived and forwarded to a printer as a
corrupt ``.gcode.3mf``. There is a second guard for that one (the ZIP
is validated before the ack), but it is a backstop, not a licence to
run the loop that needs it.
* #3001 -- ``uvloop.loop.Server`` rejects attribute assignment, which
took out every RTSP camera in 1.2.5.4. Fixed, and the fix is loop
agnostic; it is named here because it is how we learned that installs
on uvloop exist at all.
Nothing is blocked and no loop is swapped: a running server that answers
requests is worth more than a purist one that refuses to boot, and by the
time this runs uvicorn has long since chosen. The point is that the two
populations this reaches -- the Proxmox VE Helper-Scripts LXC, which writes
its own unit with no loop pinned, and native installs predating the #1896
fix, which never gained the flag because ``update.sh`` does not rewrite
unit files -- have no other way to find out. The camera outage was visible;
a truncated upload is not.
Returns True when the warning was emitted.
"""
if not running_on_uvloop(loop):
return False
logger.warning(
"Running on uvloop, which Bambuddy is not tested or shipped on. Virtual Printer FTP "
"uploads can be silently truncated on this loop (#1896). Add '%s' to the uvicorn "
"command in your service file and restart. Every installer Bambuddy ships already "
"does this; a unit written by a third-party script, or one created before 2026-07-05, "
"will not, and updating does not add it.",
_LOOP_FLAG,
)
return True
+5 -1
View File
@@ -8767,10 +8767,14 @@ async def lifespan(app: FastAPI):
# Startup
# Install Windows-only asyncio Proactor cleanup-RST filter (#1113) before
# anything else can spawn tasks that might trip it.
from backend.app.core.asyncio_handlers import install_proactor_reset_filter
from backend.app.core.asyncio_handlers import install_proactor_reset_filter, warn_if_running_on_uvloop
install_proactor_reset_filter()
# Before init_db, so the warning is near the top of the log rather than
# below a migration run. See warn_if_running_on_uvloop for what is at stake.
warn_if_running_on_uvloop()
await init_db()
# Browser download tokens expire after five minutes. Remove abandoned
+32 -5
View File
@@ -14,6 +14,7 @@ import ssl
import struct
import subprocess
import uuid
import weakref
from datetime import datetime
from pathlib import Path
@@ -55,6 +56,28 @@ _active_capture_pids: set[int] = set()
# the Obico-vs-snapshot pair from the report.
_inflight_captures: dict[str, asyncio.Task[bytes | None]] = {}
# In-flight connection handlers for each live TLS proxy server (#3001).
#
# This belongs on the server object, and for one release it lived there as an
# instance attribute. That works on asyncio's own Server and raises
# AttributeError on uvloop's, which is a Cython cdef class with no __dict__.
# Every launch path this repo ships pins --loop asyncio (added for #1896), so
# none of them could hit it -- but requirements.txt pins uvicorn[standard],
# which installs uvloop, so anything launched without that flag gets uvloop
# from --loop auto and loses every RTSP camera. That is real deployments: the
# Proxmox VE Helper-Scripts LXC writes its own unit, and native installs
# predating the #1896 pin never get it either, since update.sh does not rewrite
# unit files. So the loop is not ours to assume, and this must not depend on it.
# The test suite could not see it either: conftest builds the loop from the
# default policy, so it only ever exercised the loop where the assignment is
# legal.
#
# Keyed weakly so a server that is dropped without close_tls_proxy() -- an
# exception between create and close -- takes its entry with it. Keying on
# id(server) instead would leak those entries forever and, worse, hand a later
# server a dead one's handler set once CPython recycles the address.
_proxy_handlers: "weakref.WeakKeyDictionary[asyncio.Server, set[asyncio.Task]]" = weakref.WeakKeyDictionary()
def get_ffmpeg_path() -> str | None:
"""Find the ffmpeg executable path.
@@ -249,6 +272,8 @@ async def create_tls_proxy(target_host: str, target_port: int) -> tuple[int, "as
# pointing here and no indication that it is a teardown race rather than a
# camera fault. Holding the set also gives close_tls_proxy something to
# cancel, so shutdown stops depending on ffmpeg having dropped its end.
# The set is published in _proxy_handlers once the server exists; see the
# note there for why it is not an attribute on the server itself.
handlers: set[asyncio.Task] = set()
async def _handle(client_reader: asyncio.StreamReader, client_writer: asyncio.StreamWriter):
@@ -341,7 +366,7 @@ async def create_tls_proxy(target_host: str, target_port: int) -> tuple[int, "as
server = await asyncio.start_server(_handle, "127.0.0.1", 0)
_local_port[0] = server.sockets[0].getsockname()[1]
server._bambuddy_proxy_handlers = handlers # type: ignore[attr-defined]
_proxy_handlers[server] = handlers
logger.debug("TLS proxy for %s:%s listening on 127.0.0.1:%s", target_host, target_port, _local_port[0])
return _local_port[0], server
@@ -357,12 +382,14 @@ async def close_tls_proxy(server: "asyncio.Server") -> None:
way to guarantee no handler outlives the server that owns it.
``Server.close_clients()`` would do this natively, but it landed in Python
3.13 and Bambuddy supports 3.10, so the handler set is tracked by hand.
3.13 and Bambuddy supports 3.10, so the handler set is tracked by hand in
``_proxy_handlers``.
Safe to call on a plain ``asyncio.Server`` from anywhere else: without the
attribute it degrades to the close/wait it replaces.
Safe to call on any ``asyncio.Server`` from anywhere else: a server that
was not created here is simply absent from the registry, and this degrades
to the close/wait it replaces.
"""
handlers: set[asyncio.Task] = getattr(server, "_bambuddy_proxy_handlers", set())
handlers: set[asyncio.Task] = _proxy_handlers.pop(server, set())
server.close()
for task in list(handlers):
task.cancel()
+4 -6
View File
@@ -643,7 +643,7 @@ async def _capture_rtsp_frame(url: str, timeout: int) -> bytes | None:
try:
from urllib.parse import urlparse
from backend.app.services.camera import create_tls_proxy
from backend.app.services.camera import close_tls_proxy, create_tls_proxy
parsed = urlparse(safe_url)
target_port = parsed.port or 322
@@ -722,8 +722,7 @@ async def _capture_rtsp_frame(url: str, timeout: int) -> bytes | None:
return None
finally:
if proxy_server:
proxy_server.close()
await proxy_server.wait_closed()
await close_tls_proxy(proxy_server)
def _transcode_to_jpeg(data: bytes) -> bytes | None:
@@ -1076,7 +1075,7 @@ async def _stream_rtsp(
try:
from urllib.parse import urlparse
from backend.app.services.camera import create_tls_proxy
from backend.app.services.camera import close_tls_proxy, create_tls_proxy
parsed = urlparse(safe_url)
target_port = parsed.port or 322
@@ -1204,8 +1203,7 @@ async def _stream_rtsp(
process.kill()
await process.wait()
if proxy_server:
proxy_server.close()
await proxy_server.wait_closed()
await close_tls_proxy(proxy_server)
async def _stream_usb(
@@ -0,0 +1,77 @@
"""Bambuddy says so when it finds itself on a loop it is not shipped on (#3001).
Every unit file in this repo pins ``--loop asyncio``, added for #1896 because
uvloop's SSL layer can truncate a Virtual Printer FTP upload. Two populations
run units we do not write and so do not have the flag: the Proxmox VE
Helper-Scripts LXC, which composes its own ``ExecStart``, and native installs
created before 2026-07-05, since ``install/update.sh`` never rewrites the unit
file. #3001 -- every RTSP camera failing at once -- is how we found out those
installs exist. A truncated upload gives no such signal, so the loop now
announces itself instead of waiting for the next visible symptom.
"""
from __future__ import annotations
import asyncio
import logging
import pytest
from backend.app.core.asyncio_handlers import running_on_uvloop, warn_if_running_on_uvloop
class _FakeUvloopLoop:
"""Stands in for ``uvloop.Loop`` by module name, which is what is matched.
Lets the detection be tested on hosts without uvloop, and keeps the two
tests below honest about *how* they identify the loop: by asking the loop
what it is, not by asking whether uvloop imports.
"""
__module__ = "uvloop.loop"
async def test_the_default_loop_is_not_flagged(caplog):
"""The loop every shipped unit file selects must stay silent."""
with caplog.at_level(logging.WARNING):
assert running_on_uvloop() is False
assert warn_if_running_on_uvloop() is False
assert "uvloop" not in caplog.text
def test_uvloop_is_detected_and_warned_about(caplog):
"""The warning has to name the flag, or it is not actionable."""
loop = _FakeUvloopLoop()
assert running_on_uvloop(loop) is True # type: ignore[arg-type]
with caplog.at_level(logging.WARNING):
assert warn_if_running_on_uvloop(loop) is True # type: ignore[arg-type]
assert "--loop asyncio" in caplog.text, "the warning must state the fix, not just the diagnosis"
assert "#1896" in caplog.text, "the truncation risk is the reason this matters; cite it"
assert caplog.records[-1].levelno == logging.WARNING
def test_uvloop_is_detected_on_a_real_uvloop_loop():
"""The stand-in above is only worth having if it matches the real thing.
Not an async test on purpose: an async test inherits the session's selector
loop, which is the loop this is trying not to be.
"""
uvloop = pytest.importorskip("uvloop", reason="uvloop is a uvicorn[standard] extra; Linux only")
async def scenario() -> tuple[bool, bool]:
return running_on_uvloop(), warn_if_running_on_uvloop()
detected, warned = uvloop.run(scenario())
assert detected is True
assert warned is True
def test_no_running_loop_is_not_uvloop():
"""Called outside a loop -- ``asyncio.get_running_loop`` raises -- this must
answer False rather than propagate, since it runs during startup."""
with pytest.raises(RuntimeError):
asyncio.get_running_loop()
assert running_on_uvloop() is False
assert warn_if_running_on_uvloop() is False
@@ -0,0 +1,111 @@
"""The RTSPS proxy must survive whichever event loop production actually runs (#3001).
1.2.5.4 shipped ``server._bambuddy_proxy_handlers = handlers`` at the end of
``create_tls_proxy``. That is legal on ``asyncio.base_events.Server``, which
carries a ``__dict__``, and an outright ``AttributeError`` on
``uvloop.loop.Server``, a Cython cdef class that does not::
AttributeError: 'uvloop.loop.Server' object has no attribute
'_bambuddy_proxy_handlers' and no __dict__ for setting new attributes
Which loop you get is decided by the launch command, not by anything in the
app. Every unit file this repo ships pins ``--loop asyncio`` (added for #1896),
so none of them could hit this -- but ``requirements.txt`` pins
``uvicorn[standard]``, which installs uvloop on Linux, so any launcher without
that flag gets uvloop from ``--loop auto``. The Proxmox VE Helper-Scripts LXC
writes its own unit with no loop pinned, and native installs predating the
#1896 pin never gained it, because ``update.sh`` does not rewrite unit files.
So the loop this code runs on is not ours to assume, which is the whole reason
these tests exist. The proxy raised before opening a socket, which is why the
in-app diagnostic reported
``capture_exception`` at 0 ms while network reachability passed at 1 ms, and
why live view, snapshots and timelapse frames all went at once on every RTSP
model (X1, H2*, P2*). A1/P1 use the chamber-image protocol and return before
the proxy, so they were untouched.
The suite could not see any of it: ``conftest.event_loop`` builds its loop from
the default policy, so every async test in the repo runs on the selector loop
-- the one loop where that assignment works. Hence two tests here. The first
drives the real function on a real uvloop loop. The second states the
underlying contract without needing uvloop installed at all: the proxy must not
store anything *on* the server object, because the server it gets is not
guaranteed to accept attributes.
"""
from __future__ import annotations
import asyncio
import pytest
from backend.app.services.camera import _proxy_handlers, close_tls_proxy, create_tls_proxy
def test_create_tls_proxy_works_on_a_uvloop_loop():
"""The regression itself, on the loop that production uses.
Deliberately not an async test: the point is the loop implementation, and
an async test would inherit the session's selector loop and prove nothing.
``uvloop.run`` owns its loop start to finish.
"""
uvloop = pytest.importorskip("uvloop", reason="uvloop is a uvicorn[standard] extra; Linux only")
async def scenario() -> int:
# Port 322 is never dialled -- create_tls_proxy only binds the local
# listener; the upstream connection is opened per client handler.
port, server = await create_tls_proxy("127.0.0.1", 322)
try:
assert port > 0
assert server in _proxy_handlers, (
"handler set must be reachable from the registry, or close_tls_proxy has nothing to cancel"
)
finally:
await close_tls_proxy(server)
assert server not in _proxy_handlers, "close_tls_proxy must drop its registry entry"
return port
assert uvloop.run(scenario()) > 0
async def test_create_tls_proxy_stores_nothing_on_the_server(monkeypatch):
"""Runs everywhere, including where uvloop is not installed.
A stand-in ``Server`` with ``__slots__`` reproduces uvloop's constraint --
no ``__dict__``, so any attribute the proxy tries to attach raises. If this
fails, the code has gone back to writing on the server object.
"""
class SlottedServer:
"""Minimum of ``asyncio.Server`` that ``create_tls_proxy`` touches."""
__slots__ = ("sockets", "__weakref__")
def __init__(self, sockets):
self.sockets = sockets
def close(self):
pass
async def wait_closed(self):
pass
real_start_server = asyncio.start_server
created: list[asyncio.Server] = []
async def fake_start_server(*args, **kwargs):
"""Bind for real -- the port has to be usable -- then hide the Server."""
real = await real_start_server(*args, **kwargs)
created.append(real)
return SlottedServer(real.sockets)
monkeypatch.setattr(asyncio, "start_server", fake_start_server)
try:
port, server = await create_tls_proxy("127.0.0.1", 322)
assert port > 0
assert _proxy_handlers.get(server) == set(), "a fresh proxy has no handlers yet, but must have an entry"
await close_tls_proxy(server)
assert server not in _proxy_handlers, "close_tls_proxy must drop its registry entry"
finally:
for real in created:
real.close()
await real.wait_closed()
@@ -36,7 +36,7 @@ import ssl
import pytest
from backend.app.services.camera import close_tls_proxy, create_tls_proxy
from backend.app.services.camera import _proxy_handlers, close_tls_proxy, create_tls_proxy
@pytest.fixture(scope="module")
@@ -148,8 +148,8 @@ class TestTheHandlerIsHeldWhileItRuns:
port, proxy = await create_tls_proxy("127.0.0.1", upstream_port)
try:
_, writer = await asyncio.open_connection("127.0.0.1", port)
assert await _wait_for(lambda: len(proxy._bambuddy_proxy_handlers) == 1)
assert not next(iter(proxy._bambuddy_proxy_handlers)).done()
assert await _wait_for(lambda: len(_proxy_handlers[proxy]) == 1)
assert not next(iter(_proxy_handlers[proxy])).done()
writer.close()
finally:
@@ -166,11 +166,11 @@ class TestTheHandlerIsHeldWhileItRuns:
port, proxy = await create_tls_proxy("127.0.0.1", upstream_port)
try:
_, writer = await asyncio.open_connection("127.0.0.1", port)
assert await _wait_for(lambda: len(proxy._bambuddy_proxy_handlers) == 1)
assert await _wait_for(lambda: len(_proxy_handlers[proxy]) == 1)
writer.close()
assert await _wait_for(lambda: proxy._bambuddy_proxy_handlers == set())
assert await _wait_for(lambda: _proxy_handlers[proxy] == set())
finally:
await _close(proxy)
finally:
@@ -187,11 +187,14 @@ class TestCloseDoesNotDependOnThePeer:
try:
port, proxy = await create_tls_proxy("127.0.0.1", upstream_port)
_, writer = await asyncio.open_connection("127.0.0.1", port)
assert await _wait_for(lambda: len(proxy._bambuddy_proxy_handlers) == 1)
assert await _wait_for(lambda: len(_proxy_handlers[proxy]) == 1)
await _close(proxy)
assert proxy._bambuddy_proxy_handlers == set()
# Stronger than "the set is empty": since #3001 the handler set
# lives in a module-level registry rather than on the server, and
# close_tls_proxy retires the whole entry.
assert proxy not in _proxy_handlers
writer.close()
finally:
await _shutdown(upstream)
@@ -204,8 +207,8 @@ class TestCloseDoesNotDependOnThePeer:
try:
port, proxy = await create_tls_proxy("127.0.0.1", upstream_port)
_, writer = await asyncio.open_connection("127.0.0.1", port)
assert await _wait_for(lambda: len(proxy._bambuddy_proxy_handlers) == 1)
handler = next(iter(proxy._bambuddy_proxy_handlers))
assert await _wait_for(lambda: len(_proxy_handlers[proxy]) == 1)
handler = next(iter(_proxy_handlers[proxy]))
with caplog.at_level(logging.ERROR, logger="asyncio"):
await _close(proxy)