Better canvas cleanup, avoids segfaults on some systems with glfw+wgpu (#73)

Author: almarklein

rendercanvas/_context.py

@@ -76,3 +76,7 @@ def present(self):
 
         # This is a stub
         return {"method": "skip"}
+
+    def _release(self):
+        """Release resources. Called by the canvas when it's closed."""
+        pass

rendercanvas/_events.py

@@ -66,7 +66,7 @@ def __init__(self):
         self._event_handlers = defaultdict(list)
         self._closed = False
 
-    def _set_closed(self):
+    def _release(self):
         self._closed = True
         self._pending_events.clear()
         self._event_handlers.clear()
@@ -222,7 +222,7 @@ async def emit(self, event):
                     callback(event)
         # Close?
         if event_type == "close":
-            self._set_closed()
+            self._release()
 
     async def close(self):
         """Close the event handler.

rendercanvas/_loop.py

@@ -133,7 +133,7 @@ async def _loop_task(self):
                     for canvas in self.get_canvases():
                         if not getattr(canvas, "_rc_closed_by_loop", False):
                             canvas._rc_closed_by_loop = True
-                            canvas._rc_close()
+                            canvas.close()
                         del canvas
 
         finally:

rendercanvas/base.py

@@ -180,7 +180,7 @@ def _final_canvas_init(self):
     def __del__(self):
         # On delete, we call the custom destroy method.
         try:
-            self._rc_close()
+            self.close()
         except Exception:
             pass
         # Since this is sometimes used in a multiple inheritance, the
@@ -438,6 +438,19 @@ def get_pixel_ratio(self):
 
     def close(self):
         """Close the canvas."""
+        # Clear the draw-function, to avoid it holding onto e.g. wgpu objects.
+        self._draw_frame = None
+        # Clear the canvas context too.
+        if hasattr(self._canvas_context, "_release"):
+            # ContextInterface (and GPUCanvasContext) has _release()
+            try:
+                self._canvas_context._release()
+            except Exception:
+                pass
+        self._canvas_context = None
+        # Clean events. Should already have happened in loop, but the loop may not be running.
+        self._events._release()
+        # Let the subclass clean up.
         self._rc_close()
 
     def get_closed(self):

rendercanvas/glfw.py

@@ -11,7 +11,6 @@
 
 import sys
 import time
-import atexit
 
 import glfw
 
@@ -151,14 +150,21 @@ def enable_glfw():
     glfw._rc_alive = True
 
 
-@atexit.register
-def terminate_glfw():
-    glfw.terminate()
-    glfw._rc_alive = False
-
-
 class GlfwCanvasGroup(BaseCanvasGroup):
-    pass
+    glfw = glfw  # make sure we can access the glfw module in the __del__
+
+    def __del__(self):
+        # Because this object is used as a class attribute (on the canvas), this
+        # __del__ method gets called later than a function registed to atexit.
+        # This is important when used in combination with wgpu, where the release of the surface
+        # should happen before the termination of glfw. On some systems this can otherwiser
+        # result in a segfault, see https://github.com/pygfx/pygfx/issues/642
+        try:
+            self.glfw._rc_alive = False
+            self.glfw.terminate()
+        except Exception:
+            pass
+        super().__del__()
 
 
 class GlfwRenderCanvas(BaseRenderCanvas):
@@ -260,7 +266,7 @@ def _on_want_close(self, *args):
     def _maybe_close(self):
         if self._window is not None:
             if glfw.window_should_close(self._window):
-                self._rc_close()
+                self.close()
 
     def _set_logical_size(self, new_logical_size):
         if self._window is None:
@@ -341,10 +347,6 @@ def _rc_set_logical_size(self, width, height):
         self._set_logical_size((float(width), float(height)))
 
     def _rc_close(self):
-        if not glfw._rc_alive:
-            # May not always be able to close the proper way on system exit
-            self._window = None
-            return
         if self._window is not None:
             glfw.destroy_window(self._window)  # not just glfw.hide_window
             self._window = None
@@ -353,14 +355,13 @@ def _rc_close(self):
         # But on some systems glfw needs a bit of time to properly close the window.
         if not self._rc_canvas_group.get_canvases():
             poll_glfw_briefly(0.05)
-            # Could also terminate glfw, but we don't know if the application is using glfw in other places.
-            # terminate_glfw()
 
     def _rc_get_closed(self):
         return self._window is None
 
     def _rc_set_title(self, title):
-        glfw.set_window_title(self._window, title)
+        if self._window is not None:
+            glfw.set_window_title(self._window, title)
 
     # %% Turn glfw events into rendercanvas events
 
@@ -565,14 +566,13 @@ def _on_char(self, window, char):
 
 def poll_glfw_briefly(poll_time=0.1):
     """Briefly poll glfw for a set amount of time.
-
     Intended to work around the bug that destroyed windows sometimes hang
     around if the mainloop exits: https://github.com/glfw/glfw/issues/1766
-
     I found that 10ms is enough, but make it 100ms just in case. You should
     only run this right after your mainloop stops.
-
     """
+    if not glfw._rc_alive:
+        return
     end_time = time.perf_counter() + poll_time
     while time.perf_counter() < end_time:
         glfw.wait_events_timeout(end_time - time.perf_counter())

tests/test_loop.py

@@ -44,7 +44,7 @@ def __init__(self, refuse_close=False):
     def _rc_gui_poll(self):
         pass
 
-    def _rc_close(self):
+    def close(self):
         # Called by the loop to close a canvas
         if not self.refuse_close:
             self.is_closed = True