From a2e912c4235f41b21fda1ab477bf0ea48a41e803 Mon Sep 17 00:00:00 2001 From: 18680368135 <18680368135@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:18:56 +0000 Subject: [PATCH] Fix macOS threading hang in wait_for_playback (issue #61) Three changes address the root causes of issue #61 where wait_for_playback() hangs indefinitely on macOS: 1. Fix race condition in prepare_and_wait_for_event(): The event callback was registered before result.set_running_or_notify_cancel() was called. If an event fired in that window, set_result() raised InvalidStateError which was silently caught, causing the event to be lost and the wait to hang forever. This matches the ordering already used in prepare_and_wait_for_property(). 2. Fix _event_generator() blocking deadlock on macOS: _mpv_wait_event(handle, -1) blocks forever on macOS when the Cocoa main-loop is not running. Replace with a pipe-based wakeup mechanism using mpv_set_wakeup_callback and select() with a 100ms timeout, then drain pending events with non-blocking _mpv_wait_event(handle, 0). This makes the event thread responsive to shutdown and prevents the deadlock. 3. Add macOS platform detection: Warn users on macOS who create an MPV instance without a wid parameter that they need a running NSApplication event loop, with a code example in the docstring. Includes unit tests for the race condition fix that do not require a running libmpv instance. Fixes #61. --- mpv.py | 95 ++++++++++++++++++++++++++++---- tests/test_mpv.py | 134 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 220 insertions(+), 9 deletions(-) diff --git a/mpv.py b/mpv.py index f9bf561..c20d7dc 100644 --- a/mpv.py +++ b/mpv.py @@ -685,11 +685,54 @@ def _make_node_str_map(d): def _event_generator(handle): - while True: - event = _mpv_wait_event(handle, -1).contents - if event.event_id.value == MpvEventID.NONE: - raise StopIteration() - yield event + """Yield events from the mpv event queue. + + Uses a pipe-based wakeup mechanism to avoid deadlocks on macOS where + ``mpv_wait_event`` with an infinite timeout can block forever when the + Cocoa main-loop is not running (see issue #61). On other platforms this + also makes the event thread responsive to shutdown even when no events + arrive, because the wakeup pipe is closed on core destruction. + """ + import select as _select + + _wakeup_fd_r, _wakeup_fd_w = os.pipe() + os.set_blocking(_wakeup_fd_r, False) + + @WakeupCallback + def _wakeup_cb(_userdata): + try: + os.write(_wakeup_fd_w, b'\x00') + except OSError: + pass + + _mpv_set_wakeup_callback(handle, _wakeup_cb, None) + + try: + while True: + try: + _ready, _, _ = _select.select([_wakeup_fd_r], [], [], 0.1) + except (OSError, ValueError): + break + + if _ready: + try: + os.read(_wakeup_fd_r, 4096) + except OSError: + break + + while True: + event = _mpv_wait_event(handle, 0).contents + if event.event_id.value == MpvEventID.NONE: + break + yield event + + finally: + os.close(_wakeup_fd_r) + os.close(_wakeup_fd_w) + try: + _mpv_set_wakeup_callback(handle, WakeupCallback(), None) + except Exception: + pass def _create_null_term_cmd_arg_array(name, args): @@ -866,6 +909,21 @@ def __init__(self, *extra_mpv_flags, log_handler=None, start_event_thread=True, """Create an MPV instance. Extra arguments and extra keyword arguments will be passed to mpv as options. + + On macOS, all bundled video output drivers are Cocoa-based and require + a running ``NSApplication`` event loop. If you do not pass ``wid`` to + embed mpv into an existing window, create and run an + ``NSApplication`` before calling any blocking method such as + ``wait_for_playback``:: + + from Cocoa import NSApplication + app = NSApplication.sharedApplication() + player = mpv.MPV() + player.loadfile('video.mp4') + @player.event_callback('end-file') + def stop(evt): + app.terminate_(None) + app.run() """ self.handle = _mpv_create() @@ -882,6 +940,16 @@ def __init__(self, *extra_mpv_flags, log_handler=None, start_event_thread=True, finally: _mpv_initialize(self.handle) + # Warn macOS users who create a player without a window handle that + # blocking calls like wait_for_playback will hang without a Cocoa + # event loop. See issue #61. + if sys.platform == 'darwin' and 'wid' not in extra_mpv_opts: + warn('On macOS, mpv requires a running NSApplication event loop to ' + 'initialise its video output. Without one, calls like ' + 'wait_for_playback() may hang indefinitely. See the MPV ' + 'class docstring for a usage example. (issue #61)', + RuntimeWarning, stacklevel=2) + self.osd = _OSDPropertyProxy(self) self.file_local = _FileLocalProxy(self) self.raw = _DecoderPropertyProxy(self, identity_decoder) @@ -1119,6 +1187,19 @@ def prepare_and_wait_for_event(self, *event_types, cond=lambda evt: True, timeou """ result = Future() + # Transition the Future to RUNNING *before* registering any event + # callback. If the callback is registered first, an event delivered + # immediately (e.g. on macOS where mpv may fail to initialise video + # output and emit end_file right away) can fire before + # set_running_or_notify_cancel() runs. In that case set_result() + # raises InvalidStateError, which is silently caught, and the event + # is lost forever — causing wait_for_playback() and similar calls to + # hang indefinitely. This matches the ordering already used in + # prepare_and_wait_for_property(). See issue #61. + result.set_running_or_notify_cancel() + if catch_errors: + self._exception_futures.add(result) + @self.event_callback(*event_types) def target_handler(evt): try: @@ -1136,10 +1217,6 @@ def target_handler(evt): err_unregister = self._set_error_handler(result) try: - result.set_running_or_notify_cancel() - if catch_errors: - self._exception_futures.add(result) - yield result self.check_core_alive() diff --git a/tests/test_mpv.py b/tests/test_mpv.py index 1d04cf4..0610239 100755 --- a/tests/test_mpv.py +++ b/tests/test_mpv.py @@ -22,6 +22,7 @@ from contextlib import contextmanager import os.path import os +import sys import time from concurrent.futures import Future, InvalidStateError @@ -990,3 +991,136 @@ def t(self, *args, **kw): m.slang = 'ru' m.terminate() # needed for synchronization of event thread handler.assert_has_calls([mock.call('slang', ['jp']), mock.call('slang', ['ru'])]) + + +class TestEventWaitRaceCondition(unittest.TestCase): + """Tests for the race condition fix in prepare_and_wait_for_event (issue #61). + + These tests do not require a running libmpv instance — they use mocks to + simulate the event thread firing an event immediately during callback + registration. + """ + + def _make_mock_mpv(self, fire_immediately=True, event_data=None): + """Create a minimal mock MPV object that supports the methods used by + prepare_and_wait_for_event.""" + m = mock.MagicMock() + m._core_shutdown = False + m._exception_futures = set() + + # Simulate the event_callback decorator: when fire_immediately is True, + # the callback is invoked right away (before the context manager body + # runs), mimicking the race where an event fires during registration. + captured_callback = [] + + def fake_event_callback(*event_types): + def decorator(func): + captured_callback.append(func) + func.unregister_mpv_events = lambda: None + if fire_immediately and event_data is not None: + func(event_data) + return func + return decorator + + m.event_callback = fake_event_callback + m._set_error_handler = lambda fut: lambda: None + m.check_core_alive = lambda: None + m._captured_callback = captured_callback + return m + + def test_event_fired_during_registration_is_not_lost(self): + """When an event fires immediately during callback registration, + the Future should capture it rather than silently dropping it.""" + from concurrent.futures import Future + + event = mock.MagicMock() + event.event_id.value = mpv.MpvEventID.END_FILE + + m = self._make_mock_mpv(fire_immediately=True, event_data=event) + + # Bind the real method from the MPV class to our mock object + result_future = Future() + with mpv.MPV.prepare_and_wait_for_event(m, 'end_file', timeout=1) as result: + pass + # The event was fired during registration, so result should be set + self.assertTrue(result.done(), + "Event fired during registration was lost — Future never completed") + + def test_future_in_running_state_before_callback_registration(self): + """Verify that set_running_or_notify_cancel is called before the + event_callback decorator runs, preventing the InvalidStateError race.""" + call_order = [] + + event = mock.MagicMock() + event.event_id.value = mpv.MpvEventID.END_FILE + + m = mock.MagicMock() + m._core_shutdown = False + m._exception_futures = set() + m._set_error_handler = lambda fut: lambda: None + m.check_core_alive = lambda: None + + def fake_event_callback(*event_types): + def decorator(func): + # Record registration order, then fire the event immediately + # to prove the Future is already in RUNNING state. + call_order.append('callback_registered') + func.unregister_mpv_events = lambda: None + func(event) + return func + return decorator + + m.event_callback = fake_event_callback + + from concurrent.futures import Future as _OrigFuture + + class TrackingFuture(_OrigFuture): + def set_running_or_notify_cancel(self): + call_order.append('set_running') + return super().set_running_or_notify_cancel() + + with mock.patch.object(mpv, 'Future', TrackingFuture): + with mpv.MPV.prepare_and_wait_for_event(m, 'end_file', timeout=1): + pass + + self.assertIn('set_running', call_order) + self.assertIn('callback_registered', call_order) + self.assertLess(call_order.index('set_running'), + call_order.index('callback_registered'), + "set_running_or_notify_cancel must be called before callback registration") + + +class TestMacOSWarning(unittest.TestCase): + """Tests for macOS platform detection and warning (issue #61).""" + + @unittest.skipUnless(sys.platform == 'darwin', 'requires macOS') + def test_macos_warning_without_wid(self): + """On macOS, creating an MPV without wid should emit a RuntimeWarning.""" + import warnings as _warnings + with _warnings.catch_warnings(record=True) as w: + _warnings.simplefilter("always") + try: + m = mpv.MPV(vo='null') + m.terminate() + except OSError: + self.skipTest("libmpv not available") + self.assertTrue( + any('NSApplication' in str(warning.message) for warning in w), + "Expected NSApplication warning on macOS without wid" + ) + + def test_no_warning_on_non_darwin(self): + """On non-macOS platforms, no NSApplication warning should be emitted.""" + with mock.patch.object(sys, 'platform', 'linux'): + import warnings as _warnings + with _warnings.catch_warnings(record=True) as w: + _warnings.simplefilter("always") + try: + m = mpv.MPV(vo='null') + m.terminate() + except OSError: + self.skipTest("libmpv not available") + self.assertFalse( + any('NSApplication' in str(warning.message) for warning in w), + "NSApplication warning should not appear on non-macOS platforms" + )