Proper closing mechanics, finally

rendercanvas/_events.py

@@ -65,7 +65,11 @@ def __init__(self):
         self._pending_events = deque()
         self._event_handlers = defaultdict(list)
         self._closed = False
-        # todo: remove all handlers when closing
+
+    def _set_closed(self):
+        self._closed = True
+        self._pending_events.clear()
+        self._event_handlers.clear()
 
     def add_handler(self, *args, order: float = 0):
         """Register an event handler to receive events.
@@ -133,6 +137,8 @@ def decorator(_callback):
         return decorator(callback)
 
     def _add_handler(self, callback, order, *types):
+        if self._closed:
+            return
         self.remove_handler(callback, *types)
         for type in types:
             self._event_handlers[type].append((order, callback))
@@ -157,6 +163,8 @@ def submit(self, event):
 
         Events are emitted later by the scheduler.
         """
+        if self._closed:
+            return
         event_type = event["event_type"]
         if event_type not in EventType:
             raise ValueError(f"Submitting with invalid event_type: '{event_type}'")
@@ -202,8 +210,6 @@ async def emit(self, event):
         """
         # Collect callbacks
         event_type = event.get("event_type")
-        if event_type == "close":
-            self._closed = True
         callbacks = self._event_handlers[event_type] + self._event_handlers["*"]
         # Dispatch
         for _order, callback in callbacks:
@@ -214,10 +220,17 @@ async def emit(self, event):
                     await callback(event)
                 else:
                     callback(event)
+        # Close?
+        if event_type == "close":
+            self._set_closed()
 
-    async def _rc_canvas_close(self):
-        """Wrap up when the scheduler detects the canvas is closed/dead."""
+    async def close(self):
+        """Close the event handler.
+
+        Drops all pending events, send the close event, and disables the emitter.
+        """
         # This is a little feature because detecting a widget from closing can be tricky.
         if not self._closed:
             self._pending_events.clear()
-            await self.emit({"event_type": "close"})
+            self.submit({"event_type": "close"})
+            await self.flush()

rendercanvas/_loop.py

@@ -88,33 +88,46 @@ async def _loop_task(self):
         # Detect active loop
         self.__state = max(self.__state, 2)
 
+        # Keep track of event emitter objects
+        event_emitters = {id(c): c._events for c in self.get_canvases()}
+
         try:
             while True:
                 await sleep(0.1)
 
+                # Get list of canvases, beware to delete the list when we're done with it!
                 canvases = self.get_canvases()
 
+                # Send close event for closed canvases
+                new_event_emitters = {id(c): c._events for c in canvases}
+                closed_canvas_ids = set(event_emitters) - set(new_event_emitters)
+                for canvas_id in closed_canvas_ids:
+                    events = event_emitters[canvas_id]
+                    await events.close()
+
                 # Keep canvases alive
                 for canvas in canvases:
                     canvas._rc_gui_poll()
                     del canvas
 
+                canvas_count = len(canvases)
+                del canvases
+
                 # Should we stop?
 
-                if not canvases:
+                if canvas_count == 0:
                     # Stop when there are no more canvases
                     break
                 elif self.__should_stop >= 2:
                     # force a stop without waiting for the canvases to close
                     break
                 elif self.__should_stop:
                     # Close all remaining canvases. Loop will stop in a next iteration.
-                    for canvas in canvases:
+                    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
-                del canvases
 
         finally:
             self._stop()
@@ -124,8 +137,6 @@ def add_task(self, async_func, *args, name="unnamed"):
 
         All tasks are stopped when the loop stops.
         """
-        # todo: implement iscoroutinefunction outside of asyncio
-        # todo: test that we don't even import asyncio by default
         if not (callable(async_func) and iscoroutinefunction(async_func)):
             raise TypeError("add_task() expects an async function.")
 

rendercanvas/_scheduler.py

@@ -193,14 +193,10 @@ async def __scheduler_task(self):
             await self._async_draw_event.wait()
             last_draw_time = time.perf_counter()
 
-        # todo: sending the close event is tricky.
-        # Even if the canvas has submitted its close event, it may not be flushed yet.
-        # We can flush here, but the problem is that when all canvases are closed, the loop
-        # closes and cancels all tasks, including this one. We can write a finally-clause,
-        # so that we can do something even when being cancelled. However, we cannot await
-        # something there .... sigh!  Maybe if we require the close-handlers to be sync?
-        # self._events._rc_canvas_close()
-        await self._events.flush()
+        # Note that when the canvas is closed, we may detect it here and break from the loop.
+        # But the task may also be waiting for a draw to happen, or something else. In that case
+        # this task will be cancelled when the loop ends. In any case, this is why this is not
+        # a good place to detect the canvas getting closed, the loop does this.
 
     def on_draw(self):
         # Bookkeeping

rendercanvas/base.py

@@ -545,8 +545,16 @@ def _rc_set_logical_size(self, width, height):
     def _rc_close(self):
         """Close the canvas.
 
-        Note that ``BaseRenderCanvas`` implements the ``close()`` method, which
-        is a rather common name; it may be necessary to re-implement that too.
+        Note that ``BaseRenderCanvas`` implements the ``close()`` method, which is a
+        rather common name; it may be necessary to re-implement that too.
+
+        Backends should probably not mark the canvas as closed yet, but wait until the
+        underlying system really closes the canvas. Otherwise the loop may end before a
+        canvas gets properly cleaned up.
+
+        Backends can emit a closed event, either in this method, or when the real close
+        happens, but this is optional, since the loop detects canvases getting closed
+        and sends the close event if this has not happened yet.
         """
         pass