diff --git a/frigate/ptz/onvif.py b/frigate/ptz/onvif.py index 9301113ea8..83094ce4a9 100644 --- a/frigate/ptz/onvif.py +++ b/frigate/ptz/onvif.py @@ -1120,6 +1120,18 @@ class OnvifController: f"Camera {camera_name} is still in ONVIF 'MOVING' status." ) + async def _shutdown(self) -> None: + """Close the camera sessions and cancel the tasks running on the loop.""" + for cam_name in list(self.cams): + await self._close_camera(cam_name) + + tasks = [t for t in asyncio.all_tasks() if t is not asyncio.current_task()] + + for task in tasks: + task.cancel() + + await asyncio.gather(*tasks, return_exceptions=True) + def close(self) -> None: """Gracefully shut down the ONVIF controller.""" if not hasattr(self, "loop") or self.loop.is_closed(): @@ -1127,6 +1139,16 @@ class OnvifController: return logger.info("Exiting ONVIF controller...") + + # anything left open here is garbage collected during interpreter + # shutdown, where its warnings can no longer be logged cleanly + try: + asyncio.run_coroutine_threadsafe(self._shutdown(), self.loop).result( + timeout=5 + ) + except TimeoutError: + logger.debug("Timed out closing ONVIF sessions") + self.config_subscriber.stop() def stop_and_cleanup(): diff --git a/frigate/test/test_ptz_onvif.py b/frigate/test/test_ptz_onvif.py index 192985df7c..907f9b2923 100644 --- a/frigate/test/test_ptz_onvif.py +++ b/frigate/test/test_ptz_onvif.py @@ -15,6 +15,8 @@ Also covers the inverse direction: the ptz movement timestamps must not be writt for a camera that has autotracking off, because nothing clears them back out. """ +import asyncio +import threading import unittest from unittest.mock import AsyncMock, MagicMock @@ -234,5 +236,40 @@ class TestManualRelativeMoveMetrics(unittest.IsolatedAsyncioTestCase): ) +class TestOnvifClose(unittest.TestCase): + """close() must release everything on the loop, since whatever it leaves is + garbage collected during interpreter shutdown, where the resulting warnings + fail to log and fill the shutdown output with logging errors.""" + + def setUp(self) -> None: + self.controller = _make_controller(autotracking_enabled=False) + self.onvif = self.controller.cams[CAMERA]["onvif"] + self.onvif.close = AsyncMock() + self.controller.config_subscriber = MagicMock() + self.controller.loop = asyncio.new_event_loop() + self.controller.loop_thread = threading.Thread( + target=self.controller._run_event_loop, daemon=True + ) + self.controller.loop_thread.start() + self.addCleanup(self.controller.loop.close) + + def test_close_closes_camera_sessions(self) -> None: + self.controller.close() + + self.onvif.close.assert_awaited_once() + + def test_close_cancels_tasks_left_on_the_loop(self) -> None: + async def forever() -> None: + while True: + await asyncio.sleep(1) + + poll = asyncio.run_coroutine_threadsafe(forever(), self.controller.loop) + + self.controller.close() + + self.assertTrue(poll.cancelled()) + self.assertFalse(self.controller.loop_thread.is_alive()) + + if __name__ == "__main__": unittest.main()