Skip to content

Commit 69f627e

Browse files
author
ShresthSamyak
committed
fix(macos): address third round of Copilot review feedback
1 parent 5be9eff commit 69f627e

2 files changed

Lines changed: 32 additions & 15 deletions

File tree

mpv.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,7 @@ def _wait_for_future(self, result, timeout):
10561056

10571057
global _core_foundation
10581058
global _cf_runloop_default_mode
1059+
start_time = time.monotonic()
10591060
try:
10601061
if _core_foundation is None or _cf_runloop_default_mode is None:
10611062
_core_foundation = ctypes.cdll.LoadLibrary(ctypes.util.find_library('CoreFoundation'))
@@ -1066,7 +1067,6 @@ def _wait_for_future(self, result, timeout):
10661067

10671068
_cf_runloop_default_mode = ctypes.c_void_p.in_dll(_core_foundation, 'kCFRunLoopDefaultMode')
10681069

1069-
start_time = time.monotonic()
10701070
while not result.done():
10711071
remaining = 0
10721072
if timeout is not None:
@@ -1086,7 +1086,11 @@ def _wait_for_future(self, result, timeout):
10861086
except Exception as e:
10871087
# Fallback if CoreFoundation loading fails: use standard waiting behavior
10881088
warn(f"Failed to load CoreFoundation for mpv macOS event loop: {e}", RuntimeWarning)
1089-
return result.result(timeout)
1089+
if timeout is not None:
1090+
fallback_timeout = max(0.0, timeout - (time.monotonic() - start_time))
1091+
else:
1092+
fallback_timeout = None
1093+
return result.result(fallback_timeout)
10901094

10911095
return result.result(0 if timeout is not None else None)
10921096
else:
@@ -1175,13 +1179,13 @@ def target_handler(evt):
11751179
rv = cond(evt)
11761180
if rv:
11771181
result.set_result(rv)
1182+
except InvalidStateError: # type: ignore
1183+
pass
11781184
except Exception as e:
11791185
try:
11801186
result.set_exception(e)
11811187
except InvalidStateError:
11821188
pass
1183-
except InvalidStateError: # type: ignore
1184-
pass
11851189

11861190
err_unregister = self._set_error_handler(result)
11871191

@@ -1673,7 +1677,7 @@ def unregister_message_handler(self, target_or_handler):
16731677
if isinstance(target_or_handler, str):
16741678
del self._message_handlers[target_or_handler] # pyre-ignore
16751679
else:
1676-
for key, val in self._message_handlers.items():
1680+
for key, val in list(self._message_handlers.items()):
16771681
if val == target_or_handler:
16781682
del self._message_handlers[key] # pyre-ignore
16791683

tests/test_mpv.py

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,6 +1013,8 @@ def test_macos_wait_for_future(self):
10131013
mock_in_dll.return_value = mock.Mock()
10141014

10151015
# Reset globals from mpv module to force re-initialization
1016+
orig_cf = mpv._core_foundation
1017+
orig_mode = mpv._cf_runloop_default_mode
10161018
mpv._core_foundation = None
10171019
mpv._cf_runloop_default_mode = None
10181020

@@ -1023,12 +1025,17 @@ def resolve_future():
10231025
except InvalidStateError:
10241026
pass
10251027

1026-
threading.Thread(target=resolve_future).start()
1027-
1028-
res = self.m._wait_for_future(future, timeout=1.0)
1029-
1030-
self.assertEqual(res, 'success')
1031-
self.assertTrue(mock_cf.CFRunLoopRunInMode.called)
1028+
thread = threading.Thread(target=resolve_future)
1029+
thread.start()
1030+
try:
1031+
res = self.m._wait_for_future(future, timeout=1.0)
1032+
1033+
self.assertEqual(res, 'success')
1034+
self.assertTrue(mock_cf.CFRunLoopRunInMode.called)
1035+
finally:
1036+
thread.join(timeout=1.0)
1037+
mpv._core_foundation = orig_cf
1038+
mpv._cf_runloop_default_mode = orig_mode
10321039

10331040
@mock.patch('sys.platform', 'darwin')
10341041
def test_macos_wait_for_future_timeout(self):
@@ -1048,11 +1055,17 @@ def test_macos_wait_for_future_timeout(self):
10481055
mock_load_library.return_value = mock_cf
10491056
mock_in_dll.return_value = mock.Mock()
10501057

1058+
orig_cf = mpv._core_foundation
1059+
orig_mode = mpv._cf_runloop_default_mode
10511060
mpv._core_foundation = None
10521061
mpv._cf_runloop_default_mode = None
10531062

10541063
from concurrent.futures import TimeoutError
1055-
with self.assertRaises(TimeoutError):
1056-
self.m._wait_for_future(future, timeout=0.1)
1057-
1058-
self.assertTrue(mock_cf.CFRunLoopRunInMode.called)
1064+
try:
1065+
with self.assertRaises(TimeoutError):
1066+
self.m._wait_for_future(future, timeout=0.1)
1067+
1068+
self.assertTrue(mock_cf.CFRunLoopRunInMode.called)
1069+
finally:
1070+
mpv._core_foundation = orig_cf
1071+
mpv._cf_runloop_default_mode = orig_mode

0 commit comments

Comments
 (0)