Fix macOS threading hang in wait_for_playback (issue #61)

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()

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"
+                )