Fix macOS threading hang in wait_for_playback (issue #61) - #316
Open
18680368135 wants to merge 1 commit into
Open
Fix macOS threading hang in wait_for_playback (issue #61)#31618680368135 wants to merge 1 commit into
18680368135 wants to merge 1 commit into
Conversation
Three changes address the root causes of issue jaseg#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 jaseg#61.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes issue #61 where
wait_for_playback()hangs indefinitely on macOS. The fix addresses three root causes:1. 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()raisedInvalidStateErrorwhich was silently caught, causing the event to be lost forever and the wait to hang indefinitely.This matches the ordering already used in
prepare_and_wait_for_property()(lines 1070-1072), whereset_running_or_notify_cancel()is called beforeobserve_property().2. Blocking deadlock in
_event_generator()on macOS_mpv_wait_event(handle, -1)blocks forever on macOS when the Cocoa main-loop is not running (because mpv cannot initialize its Cocoa-based video output drivers without anNSApplicationevent loop).Replaced with a pipe-based wakeup mechanism using
mpv_set_wakeup_callbackandselect()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. The wakeup callback infrastructure (WakeupCallback,_mpv_set_wakeup_callback) was already defined in the codebase but unused.3. macOS platform detection and warning
Added a
RuntimeWarningwhen creating anMPVinstance on macOS without awidparameter, explaining that a runningNSApplicationevent loop is required. The docstring includes a code example showing how to set up the Cocoa event loop.Testing
test_event_fired_during_registration_is_not_lost: Verifies that events fired during callback registration are captured (not lost due toInvalidStateError)test_future_in_running_state_before_callback_registration: Verifies thatset_running_or_notify_cancel()is called before callback registrationtest_macos_warning_without_wid: Verifies the macOS warning is emitted (skipped on non-macOS)test_no_warning_on_non_darwin: Verifies no spurious warning on non-macOS platformsAll tests pass on Linux. The race condition tests use mocks and do not require a running libmpv instance.
Fixes #61