MNT/FIX: macosx change Timer to NSTimer instance

greglucas · greglucas · commit a0e50b2b6487 · 2023-05-31T21:20:31.000-06:00
Newer macos frameworks now support blocks, so we can take advantage of
that and inline our method call while creating and scheduling the timer
all at once. This removes some of the reference counting and alloc/dealloc
record keeping.

In the mac framework, an interval of 0 will only fire once, so if we
aren't a singleshot timer we need to ensure that the interval is
greater than 0.
diff --git a/lib/matplotlib/backends/backend_macosx.py b/lib/matplotlib/backends/backend_macosx.py
@@ -69,6 +69,7 @@ def callback_func(callback, timer):
             self._timers.remove(timer)
             timer.stop()
         timer = self.new_timer(interval=0)
+        timer.single_shot = True
         timer.add_callback(callback_func, callback, timer)
         self._timers.add(timer)
         timer.start()
diff --git a/lib/matplotlib/tests/test_backend_macosx.py b/lib/matplotlib/tests/test_backend_macosx.py
@@ -1,4 +1,5 @@
 import os
+from unittest.mock import Mock
 
 import pytest
 
@@ -44,3 +45,26 @@ def new_choose_save_file(title, directory, filename):
         # Check the savefig.directory rcParam got updated because
         # we added a subdirectory "test"
         assert mpl.rcParams["savefig.directory"] == f"{tmp_path}/test"
+
+
+@pytest.mark.backend('macosx')
+def test_timers():
+    # A timer with <1 millisecond gets converted to int and therefore 0
+    # milliseconds, which the mac framework interprets as singleshot.
+    # We only want singleshot if we specify that ourselves, otherwise we want
+    # a repeating timer
+    fig = plt.figure()
+    timer = fig.canvas.new_timer(0.1)
+    mock = Mock()
+    timer.add_callback(mock)
+    timer.start()
+    plt.pause(0.1)
+    timer.stop()
+    assert mock.call_count > 1
+
+    # Now turn it into a single shot timer and verify only one gets triggered
+    mock.call_count = 0
+    timer.single_shot = True
+    timer.start()
+    plt.pause(0.1)
+    assert mock.call_count == 1
diff --git a/src/_macosx.m b/src/_macosx.m
@@ -1811,93 +1811,53 @@ - (void)flagsChanged:(NSEvent *)event
 
 typedef struct {
     PyObject_HEAD
-    CFRunLoopTimerRef timer;
+    NSTimer* timer;
+
 } Timer;
 
 static PyObject*
 Timer_new(PyTypeObject* type, PyObject *args, PyObject *kwds)
 {
     lazy_init();
     Timer* self = (Timer*)type->tp_alloc(type, 0);
-    if (!self) { return NULL; }
+    if (!self) {
+        return NULL;
+    }
     self->timer = NULL;
     return (PyObject*) self;
 }
 
 static PyObject*
 Timer_repr(Timer* self)
 {
-    return PyUnicode_FromFormat("Timer object %p wrapping CFRunLoopTimerRef %p",
+    return PyUnicode_FromFormat("Timer object %p wrapping NSTimerRef %p",
                                (void*) self, (void*)(self->timer));
 }
 
-static void timer_callback(CFRunLoopTimerRef timer, void* info)
-{
-    gil_call_method(info, "_on_timer");
-}
-
-static void context_cleanup(const void* info)
-{
-    Py_DECREF((PyObject*)info);
-}
-
 static PyObject*
 Timer__timer_start(Timer* self, PyObject* args)
 {
-    CFRunLoopRef runloop;
-    CFRunLoopTimerRef timer;
-    CFRunLoopTimerContext context;
-    CFAbsoluteTime firstFire;
-    CFTimeInterval interval;
+    NSTimeInterval interval;
     PyObject* py_interval = NULL, * py_single = NULL, * py_on_timer = NULL;
     int single;
-    runloop = CFRunLoopGetMain();
-    if (!runloop) {
-        PyErr_SetString(PyExc_RuntimeError, "Failed to obtain run loop");
-        return NULL;
-    }
     if (!(py_interval = PyObject_GetAttrString((PyObject*)self, "_interval"))
         || ((interval = PyFloat_AsDouble(py_interval) / 1000.), PyErr_Occurred())
         || !(py_single = PyObject_GetAttrString((PyObject*)self, "_single"))
         || ((single = PyObject_IsTrue(py_single)) == -1)
         || !(py_on_timer = PyObject_GetAttrString((PyObject*)self, "_on_timer"))) {
         goto exit;
     }
-    // (current time + interval) is time of first fire.
-    firstFire = CFAbsoluteTimeGetCurrent() + interval;
-    if (single) {
-        interval = 0;
-    }
     if (!PyMethod_Check(py_on_timer)) {
         PyErr_SetString(PyExc_RuntimeError, "_on_timer should be a Python method");
         goto exit;
     }
-    Py_INCREF(self);
-    context.version = 0;
-    context.retain = NULL;
-    context.release = context_cleanup;
-    context.copyDescription = NULL;
-    context.info = self;
-    timer = CFRunLoopTimerCreate(kCFAllocatorDefault,
-                                 firstFire,
-                                 interval,
-                                 0,
-                                 0,
-                                 timer_callback,
-                                 &context);
-    if (!timer) {
-        PyErr_SetString(PyExc_RuntimeError, "Failed to create timer");
-        goto exit;
-    }
-    if (self->timer) {
-        CFRunLoopTimerInvalidate(self->timer);
-        CFRelease(self->timer);
-    }
-    CFRunLoopAddTimer(runloop, timer, kCFRunLoopCommonModes);
-    /* Don't release the timer here, since the run loop may be destroyed and
-     * the timer lost before we have a chance to decrease the reference count
-     * of the attribute */
-    self->timer = timer;
+
+    // hold a reference to the timer so we can invalidate/stop it later
+    self->timer = [NSTimer scheduledTimerWithTimeInterval: interval
+                                            repeats: !single
+                                              block: ^(NSTimer *timer) {
+        gil_call_method((PyObject*)self, "_on_timer");
+    }];
 exit:
     Py_XDECREF(py_interval);
     Py_XDECREF(py_single);
@@ -1913,8 +1873,7 @@ static void context_cleanup(const void* info)
 Timer__timer_stop(Timer* self)
 {
     if (self->timer) {
-        CFRunLoopTimerInvalidate(self->timer);
-        CFRelease(self->timer);
+        [self->timer invalidate];
         self->timer = NULL;
     }
     Py_RETURN_NONE;
@@ -1935,7 +1894,7 @@ static void context_cleanup(const void* info)
     .tp_repr = (reprfunc)Timer_repr,
     .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
     .tp_new = (newfunc)Timer_new,
-    .tp_doc = "A Timer object wraps a CFRunLoopTimerRef and can add it to the event loop.",
+    .tp_doc = "A Timer object that contains an NSTimer that gets added to the event loop when started.",
     .tp_methods = (PyMethodDef[]){  // All docstrings are inherited.
         {"_timer_start",
          (PyCFunction)Timer__timer_start,
