Refactor loop/canvas closing a bit

rendercanvas/_loop.py

@@ -6,12 +6,21 @@
 import signal
 import weakref
 
-from ._coreutils import log_exception, BaseEnum
+from ._coreutils import logger, log_exception, BaseEnum
+
 
 # Note: technically, we could have a global loop proxy object that defers to any of the other loops.
 # That would e.g. allow using glfw with qt together. Probably a too weird use-case for the added complexity.
 
 
+HANDLED_SIGNALS = (
+    signal.SIGINT,  # Unix signal 2. Sent by Ctrl+C.
+    signal.SIGTERM,  # Unix signal 15. Sent by `kill <pid>`.
+)
+# if sys.platform == "win32":
+#     HANDLED_SIGNALS += (signal.SIGBREAK,)  # Windows signal 21. Sent by Ctrl+Break.
+
+
 class BaseTimer:
     """The Base class for a timer object.
 
@@ -143,9 +152,9 @@ class BaseLoop:
     _TimerClass = None  # subclases must set this
 
     def __init__(self):
-        self._schedulers = set()
+        self._schedulers = []
         self._is_inside_run = False
-        self._stop_when_no_canvases = False
+        self._should_stop = 0
 
         # The loop object runs a lightweight timer for a few reasons:
         # * Support running the loop without windows (e.g. to keep animations going).
@@ -158,27 +167,52 @@ def __init__(self):
 
     def _register_scheduler(self, scheduler):
         # Gets called whenever a canvas in instantiated
-        self._schedulers.add(scheduler)
+        self._schedulers.append(scheduler)
         self._gui_timer.start(0.1)  # (re)start our internal timer
 
+    def get_canvases(self):
+        """Get a list of currently active (not-closed) canvases."""
+        canvases = []
+        schedulers = []
+
+        for scheduler in self._schedulers:
+            canvas = scheduler._get_canvas()
+            if canvas is not None:
+                canvases.append(canvas)
+                schedulers.append(scheduler)
+
+        # Forget schedulers that no longer have a live canvas
+        self._schedulers = schedulers
+
+        return canvases
+
     def _tick(self):
         # Keep the GUI alive on every tick
         self._rc_gui_poll()
 
-        # Check all schedulers
-        schedulers_to_close = []
-        for scheduler in self._schedulers:
-            if scheduler._get_canvas() is None:
-                schedulers_to_close.append(scheduler)
+        # Clean internal schedulers list
+        self.get_canvases()
 
-        # Forget schedulers that no longer have an live canvas
-        for scheduler in schedulers_to_close:
-            self._schedulers.discard(scheduler)
+        # Our loop can still tick, even if the loop is not started via our run() method.
+        # If this is the case, we don't run the close/stop logic
+        if not self._is_inside_run:
+            return
 
-        # Check whether we must stop the loop. Note that if the loop is not
-        # actually running, but is in an interactive mode, this has no effect.
-        if self._stop_when_no_canvases and not self._schedulers:
-            self.stop()
+        # Should we stop?
+
+        if not self._schedulers:
+            # Stop when there are no more canvases
+            self._rc_stop()
+        elif self._should_stop >= 2:
+            # force a stop without waiting for the canvases to close
+            self._rc_stop()
+        elif self._should_stop:
+            # Close all remaining canvases. Loop will stop in a next iteration.
+            for canvas in self.get_canvases():
+                if not getattr(canvas, "_rc_closed_by_loop", False):
+                    canvas._rc_closed_by_loop = True
+                    canvas._rc_close()
+                    del canvas
 
     def call_soon(self, callback, *args):
         """Arrange for a callback to be called as soon as possible.
@@ -216,47 +250,71 @@ def call_repeated(self, interval, callback, *args):
         timer.start()
         return timer
 
-    def run(self, stop_when_no_canvases=True):
+    def run(self):
         """Enter the main loop.
 
         This provides a generic API to start the loop. When building an application (e.g. with Qt)
         its fine to start the loop in the normal way.
         """
+        # Note that when the loop is started via this method, we always stop
+        # when the last canvas is closed. Keeping the loop alive is a use-case
+        # for interactive sessions, where the loop is already running, or started
+        # "behind our back". So we don't need to accomodate for this.
 
         # Cannot run if already running
         if self._is_inside_run:
             raise RuntimeError("loop.run() is not reentrant.")
 
-        self._stop_when_no_canvases = bool(stop_when_no_canvases)
+        # Register interrupt handler
+        prev_sig_handlers = self.__setup_interrupt()
 
-        # Handle interrupts
-        try:
-            prev_int_handler = signal.signal(signal.SIGINT, self.__on_interrupt)
-        except ValueError:
-            prev_int_handler = None
-
-        # Run. We could be in this loop for a loong time. Or we can
-        # exit emmidiately if the backend already has an (interactive)
-        # event loop. In the latter case, see how we disable the signal
-        # again, so we don't interfere with that loop.
+        # Run. We could be in this loop for a long time. Or we can exit
+        # immediately if the backend already has an (interactive) event
+        # loop. In the latter case, note how we restore the sigint
+        # handler again, so we don't interfere with that loop.
         self._is_inside_run = True
         try:
             self._rc_run()
-        except KeyboardInterrupt:
-            pass
         finally:
             self._is_inside_run = False
-            if prev_int_handler is not None:
-                signal.signal(signal.SIGINT, prev_int_handler)
+            for sig, cb in prev_sig_handlers.items():
+                signal.signal(sig, cb)
 
     def stop(self):
-        """Stop the currently running event loop."""
+        """Close all windows and stop the currently running event loop.
+
+        This only has effect when the event loop is currently running via ``.run()``.
+        I.e. not when a Qt app is started with ``app.exec()``, or when Qt or asyncio
+        is running interactively in your IDE.
+        """
         # Only take action when we're inside the run() method
         if self._is_inside_run:
-            self._rc_stop()
+            self._should_stop += 1
+            if self._should_stop >= 4:
+                # If for some reason the tick method is no longer being called, but the loop is still running, we can still stop it by spamming stop() :)
+                self._rc_stop()
+
+    def __setup_interrupt(self):
+        def on_interrupt(sig, _frame):
+            logger.warning(f"Received signal {signal.strsignal(sig)}")
+            self.stop()
 
-    def __on_interrupt(self, *args):
-        self.stop()
+        prev_handlers = {}
+
+        for sig in HANDLED_SIGNALS:
+            prev_handler = signal.getsignal(sig)
+            if prev_handler in (None, signal.SIG_IGN, signal.SIG_DFL):
+                # Only register if the old handler for SIGINT was not None,
+                # which means that a non-python handler was installed, i.e. in
+                # Julia, and not SIG_IGN which means we should ignore the interrupts.
+                pass
+            else:
+                # Setting the signal can raise ValueError if this is not the main thread/interpreter
+                try:
+                    prev_handlers[sig] = signal.signal(signal.SIGINT, on_interrupt)
+                except ValueError:
+                    break
+        return prev_handlers
 
     def _rc_run(self):
         """Start running the event-loop.

rendercanvas/base.py

@@ -116,9 +116,9 @@ def _final_canvas_init(self):
             self.set_title(kwargs["title"])
 
     def __del__(self):
-        # On delete, we call the custom close method.
+        # On delete, we call the custom destroy method.
         try:
-            self.close()
+            self._rc_close()
         except Exception:
             pass
         # Since this is sometimes used in a multiple inheritance, the
@@ -497,9 +497,6 @@ def _rc_set_logical_size(self, width, height):
     def _rc_close(self):
         """Close the canvas.
 
-        For widgets that are wrapped by a ``WrapperRenderCanvas``, this should probably
-        close the wrapper instead.
-
         Note that ``BaseRenderCanvas`` implements the ``close()`` method, which
         is a rather common name; it may be necessary to re-implement that too.
         """
@@ -512,9 +509,6 @@ def _rc_is_closed(self):
     def _rc_set_title(self, title):
         """Set the canvas title. May be ignored when it makes no sense.
 
-        For widgets that are wrapped by a ``WrapperRenderCanvas``, this should probably
-        set the title of the wrapper instead.
-
         The default implementation does nothing.
         """
         pass

rendercanvas/glfw.py

@@ -174,7 +174,7 @@ def __init__(self, *args, present_method=None, **kwargs):
         # Register callbacks. We may get notified too often, but that's
         # ok, they'll result in a single draw.
         glfw.set_framebuffer_size_callback(self._window, weakbind(self._on_size_change))
-        glfw.set_window_close_callback(self._window, weakbind(self._check_close))
+        glfw.set_window_close_callback(self._window, weakbind(self._on_want_close))
         glfw.set_window_refresh_callback(self._window, weakbind(self._on_window_dirty))
         glfw.set_window_focus_callback(self._window, weakbind(self._on_window_dirty))
         set_window_content_scale_callback(
@@ -230,6 +230,16 @@ def _determine_size(self):
         }
         self.submit_event(ev)
 
+    def _on_want_close(self, *args):
+        # Called when the user attempts to close the window, for example by clicking the close widget in the title bar.
+        # We could prevent closing the window here. But we don't :)
+        pass  # Prevent closing: glfw.set_window_should_close(self._window, 0)
+
+    def _maybe_close(self):
+        if self._window is not None:
+            if glfw.window_should_close(self._window):
+                self._rc_close()
+
     # %% Methods to implement RenderCanvas
 
     def _set_logical_size(self, new_logical_size):
@@ -305,8 +315,9 @@ def _rc_close(self):
         if not loop._glfw_initialized:
             return  # glfw is probably already terminated
         if self._window is not None:
-            glfw.set_window_should_close(self._window, True)
-            self._check_close()
+            glfw.destroy_window(self._window)  # not just glfw.hide_window
+            self._window = None
+            self.submit_event({"event_type": "close"})
 
     def _rc_is_closed(self):
         return self._window is None
@@ -330,19 +341,6 @@ def _on_size_change(self, *args):
         self._determine_size()
         self.request_draw()
 
-    def _check_close(self, *args):
-        # Follow the close flow that glfw intended.
-        # This method can be overloaded and the close-flag can be set to False
-        # using set_window_should_close() if now is not a good time to close.
-        if self._window is not None and glfw.window_should_close(self._window):
-            self._on_close()
-
-    def _on_close(self, *args):
-        if self._window is not None:
-            glfw.destroy_window(self._window)  # not just glfw.hide_window
-            self._window = None
-            self.submit_event({"event_type": "close"})
-
     def _on_mouse_button(self, window, but, action, mods):
         # Map button being changed, which we use to update self._pointer_buttons.
         button_map = {
@@ -539,6 +537,9 @@ def _terminate_glfw(self):
         glfw.terminate()
 
     def _rc_gui_poll(self):
+        for canvas in self.get_canvases():
+            canvas._maybe_close()
+            del canvas
         glfw.post_empty_event()  # Awake the event loop, if it's in wait-mode
         glfw.poll_events()