MAINT: Add GC tests (#66)

pytest.ini

@@ -1,5 +1,7 @@
 [pytest]
 junit_family=legacy
+addopts =
+    --durations=10 -ra --cov-report= --tb=short
 filterwarnings =
     error
     ignore::ResourceWarning

pyvistaqt/editor.py

@@ -1,5 +1,6 @@
 """This module contains the Qt scene editor."""
 
+import weakref
 from typing import List
 
 from pyvista import Renderer
@@ -28,6 +29,7 @@ def __init__(self, parent: MainWindow, renderers: List[Renderer]) -> None:
         """Initialize the Editor."""
         super().__init__(parent=parent)
         self.renderers = renderers
+        del renderers
 
         self.tree_widget = QTreeWidget()
         self.tree_widget.setHeaderHidden(True)
@@ -78,19 +80,25 @@ def toggle(self) -> None:
 def _get_renderer_widget(renderer: Renderer) -> QWidget:
     widget = QWidget()
     layout = QVBoxLayout()
+    axes = QCheckBox("Axes")
+    if hasattr(renderer, "axes_widget"):
+        axes.setChecked(renderer.axes_widget.GetEnabled())
+    else:
+        axes.setChecked(False)
+
+    renderer_ref = weakref.ref(renderer)
+    del renderer
 
     # axes
     def _axes_callback(state: bool) -> None:
+        renderer = renderer_ref()
+        if renderer is None:  # pragma: no cover
+            return
         if state:
             renderer.show_axes()
         else:
             renderer.hide_axes()
 
-    axes = QCheckBox("Axes")
-    if hasattr(renderer, "axes_widget"):
-        axes.setChecked(renderer.axes_widget.GetEnabled())
-    else:
-        axes.setChecked(False)
     axes.toggled.connect(_axes_callback)
     layout.addWidget(axes)
 
@@ -105,9 +113,16 @@ def _get_actor_widget(actor: vtkActor) -> QWidget:
     prop = actor.GetProperty()
 
     # visibility
+    set_vis_ref = weakref.ref(actor.SetVisibility)
+
+    def _set_vis(visibility: bool) -> None:  # pragma: no cover
+        set_vis = set_vis_ref()
+        if set_vis is not None:
+            set_vis(visibility)
+
     visibility = QCheckBox("Visibility")
     visibility.setChecked(actor.GetVisibility())
-    visibility.toggled.connect(actor.SetVisibility)
+    visibility.toggled.connect(_set_vis)
     layout.addWidget(visibility)
 
     if prop is not None:
@@ -116,7 +131,14 @@ def _get_actor_widget(actor: vtkActor) -> QWidget:
         opacity = QDoubleSpinBox()
         opacity.setMaximum(1.0)
         opacity.setValue(prop.GetOpacity())
-        opacity.valueChanged.connect(prop.SetOpacity)
+        set_opacity_ref = weakref.ref(prop.SetOpacity)
+
+        def _set_opacity(opacity: float) -> None:  # pragma: no cover
+            set_opacity = set_opacity_ref()
+            if set_opacity is not None:
+                set_opacity(opacity)
+
+        opacity.valueChanged.connect(_set_opacity)
         tmp_layout.addWidget(QLabel("Opacity"))
         tmp_layout.addWidget(opacity)
         layout.addLayout(tmp_layout)

pyvistaqt/plotting.py

@@ -311,7 +311,10 @@ def _render(self, *args: Any, **kwargs: Any) -> BasePlotter.render:
     @conditional_decorator(threaded, platform.system() == "Darwin")
     def render(self) -> None:
         """Override the ``render`` method to handle threading issues."""
-        return self.render_signal.emit()
+        try:
+            return self.render_signal.emit()
+        except RuntimeError:  #  wrapped C/C++ object has been deleted
+            return None
 
     @wraps(BasePlotter.enable)
     def enable(self) -> None:
@@ -400,6 +403,16 @@ def close(self) -> None:
             self.render_timer.stop()
         BasePlotter.close(self)
         QVTKRenderWindowInteractor.close(self)
+        # Qt LeaveEvent requires _Iren so we use _FakeIren instead of None
+        # to resolve the ref to vtkGenericRenderWindowInteractor
+        self._Iren = (  # pylint: disable=invalid-name,attribute-defined-outside-init
+            _FakeEventHandler()
+        )
+        for key in ("_RenderWindow", "renderer"):
+            try:
+                setattr(self, key, None)
+            except AttributeError:
+                pass
 
 
 class BackgroundPlotter(QtInteractor):
@@ -1013,3 +1026,14 @@ def __getitem__(self, idx: Tuple[int, int]) -> Optional[BackgroundPlotter]:
         row, col = idx
         self._plotter = self._plotters[row * self._ncols + col]
         return self._plotter
+
+
+class _FakeEventHandler:
+    # pylint: disable=too-few-public-methods
+
+    def _noop(self, *args: tuple, **kwargs: dict) -> None:
+        pass
+
+    SetDPI = EnterEvent = MouseMoveEvent = LeaveEvent = SetSize = _noop
+    SetEventInformation = ConfigureEvent = SetEventInformationFlipY = _noop
+    KeyReleaseEvent = CharEvent = _noop

tests/conftest.py

@@ -1,13 +1,34 @@
+import gc
 import importlib
+import inspect
 import sys
 
 import pytest
+import pyvista
 from pyvista.plotting import system_supports_plotting
 import pyvistaqt
 
 NO_PLOTTING = not system_supports_plotting()
 
 
+def pytest_configure(config):
+    """Configure pytest options."""
+    # Fixtures
+    for fixture in ('check_gc',):
+        config.addinivalue_line('usefixtures', fixture)
+    # Markers
+    for marker in ('allow_bad_gc', 'allow_bad_gc_pyside'):
+        config.addinivalue_line('markers', marker)
+
+
+# Adapted from PyVista
+def _is_vtk(obj):
+    try:
+        return obj.__class__.__name__.startswith('vtk')
+    except Exception:  # old Python sometimes no __class__.__name__
+        return False
+
+
 def _check_qt_installed():
     try:
         from qtpy import QtCore  # noqa
@@ -17,6 +38,65 @@ def _check_qt_installed():
         return True
 
 
+@pytest.fixture(autouse=True)
+def check_gc(request):
+    """Ensure that all VTK objects are garbage-collected by Python."""
+    if 'test_ipython' in request.node.name:  # XXX this keeps a ref
+        yield
+        return
+    try:
+        from qtpy import API_NAME
+    except Exception:
+        API_NAME = ''
+    marks = set(mark.name for mark in request.node.iter_markers())
+    if 'allow_bad_gc' in marks:
+        yield
+        return
+    if 'allow_bad_gc_pyside' in marks and API_NAME.lower().startswith('pyside'):
+        yield
+        return
+    gc.collect()
+    before = set(id(o) for o in gc.get_objects() if _is_vtk(o))
+    yield
+    pyvista.close_all()
+    gc.collect()
+    after = [
+        o
+        for o in gc.get_objects()
+        if _is_vtk(o) and id(o) not in before
+    ]
+    msg = 'Not all objects GCed:\n'
+    for obj in after:
+        cn = obj.__class__.__name__
+        cf = inspect.currentframe()
+        referrers = [
+            v for v in gc.get_referrers(obj)
+            if v is not after and v is not cf
+        ]
+        del cf
+        for ri, referrer in enumerate(referrers):
+            if isinstance(referrer, dict):
+                for k, v in referrer.items():
+                    if k is obj:
+                        referrers[ri] = 'dict: d key'
+                        del k, v
+                        break
+                    elif v is obj:
+                        referrers[ri] = f'dict: d[{k!r}]'
+                        #raise RuntimeError(referrers[ri])
+                        del k, v
+                        break
+                    del k, v
+                else:
+                    referrers[ri] = f'dict: len={len(referrer)}'
+            else:
+                referrers[ri] = repr(referrer)
+            del ri, referrer
+        msg += f'{cn}: {referrers}\n'
+        del cn, referrers
+    assert len(after) == 0, msg
+
+
 @pytest.fixture()
 def plotting():
     """Require plotting."""

tests/test_plotting.py

@@ -1,6 +1,7 @@
 import os
 from packaging.version import Version
 import platform
+import weakref
 
 import numpy as np
 import pytest
@@ -197,6 +198,14 @@ def test_counter(qtbot):
     assert counter.count == 0
 
 
+# TODO: Fix gc on PySide6
+@pytest.mark.parametrize('border', (True, False))
+@pytest.mark.allow_bad_gc_pyside
+def test_subplot_gc(border):
+    BackgroundPlotter(shape=(2, 1), update_app_icon=False, border=border)
+
+
+@pytest.mark.allow_bad_gc_pyside
 def test_editor(qtbot, plotting):
     # test editor=False
     plotter = BackgroundPlotter(editor=False, off_screen=False)
@@ -226,7 +235,8 @@ def test_editor(qtbot, plotting):
 
     # add at least an actor
     plotter.subplot(0, 0)
-    plotter.add_mesh(pyvista.Sphere())
+    pd = pyvista.Sphere()
+    actor = plotter.add_mesh(pd)
     plotter.subplot(1, 0)
     plotter.show_axes()
 
@@ -258,6 +268,7 @@ def test_editor(qtbot, plotting):
 
     # hide the editor for coverage
     editor.toggle()
+    plotter.remove_actor(actor)
     plotter.close()
 
 
@@ -435,6 +446,7 @@ def _to_array(camera_position):
     with pytest.raises(TypeError, match=match):
         plotter_one.link_views_across_plotters(plotter_two, other_views=[0.0])
 
+
 @pytest.mark.parametrize('show_plotter', [
     True,
     False,
@@ -583,6 +595,8 @@ def test_background_plotting_toolbar(qtbot, plotting):
     plotter.close()
 
 
+# TODO: _render_passes not GC'ed
+@pytest.mark.allow_bad_gc_pyside
 @pytest.mark.skipif(
     platform.system() == 'Windows', reason='Segfaults on Windows')
 def test_background_plotting_menu_bar(qtbot, plotting):
@@ -641,7 +655,9 @@ def test_drop_event(tmpdir, qtbot):
     mesh = pyvista.Cone()
     mesh.save(filename)
     assert os.path.isfile(filename)
-    plotter = BackgroundPlotter(update_app_icon=False, show=True)
+    plotter = BackgroundPlotter(update_app_icon=False)
+    with qtbot.wait_exposed(plotter.app_window, timeout=10000):
+        plotter.app_window.show()
     point = QPointF(0, 0)
     data = QMimeData()
     data.setUrls([QUrl(filename)])
@@ -678,7 +694,9 @@ def test_drag_event(tmpdir):
 
 
 def test_gesture_event(qtbot):
-    plotter = BackgroundPlotter()
+    plotter = BackgroundPlotter(update_app_icon=False)
+    with qtbot.wait_exposed(plotter.app_window, timeout=10000):
+        plotter.app_window.show()
     gestures = [QPinchGesture()]
     event = QGestureEvent(gestures)
     plotter.gesture_event(event)
@@ -688,10 +706,10 @@ def test_gesture_event(qtbot):
 def test_background_plotting_add_callback(qtbot, monkeypatch, plotting):
     class CallBack(object):
         def __init__(self, sphere):
-            self.sphere = sphere
+            self.sphere = weakref.ref(sphere)
 
         def __call__(self):
-            self.sphere.points *= 0.5
+            self.sphere().points[:] = self.sphere().points * 0.5
 
     update_count = [0]
     orig_update_app_icon = BackgroundPlotter.update_app_icon
@@ -766,10 +784,23 @@ def update_app_icon(slf):
     assert not callback_timer.isActive()  # window stops the callback
 
 
+def allow_bad_gc_old_pyvista(func):
+    if Version(pyvista.__version__) < Version('0.37'):
+        return pytest.mark.allow_bad_gc(func)
+    else:
+        return func
+
+
+# TODO: Need to fix this allow_bad_gc:
+# - the actors are not cleaned up in the non-empty scene case
+# - the q_key_press leaves a lingering vtkUnsignedCharArray referred to by
+#   a "managedbuffer" object
+@allow_bad_gc_old_pyvista
+@pytest.mark.allow_bad_gc_pyside
 @pytest.mark.parametrize('close_event', [
     "plotter_close",
     "window_close",
-    "q_key_press",
+    pytest.param("q_key_press", marks=pytest.mark.allow_bad_gc),
     "menu_exit",
     "del_finalizer",
     ])