fix: a render waiting on the lock must not interleave with close()

reacton/core.py

@@ -1322,29 +1322,33 @@ def collect(context: ComponentContext):
             self._remove_element(self.element, default_key="/", parent_key=ROOT_KEY)
             logger.info("Removing elements done.")
             assert self.context is self.context_root
-        if self.container:
-            self.container.close()
-            if isinstance(self.container, widgets.DOMWidget) and self.container.layout is not None:
-                self.container.layout.close()
-        if self._shared_elements:
-            raise RuntimeError(f"Element not cleaned up: {self._shared_elements}")
-        if self._orphans:
-            orphan_widgets = set([_get_widgets_dict()[k] for k in self._orphans])
-            raise RuntimeError(f"Orphan widgets not cleaned up for widgets: {orphan_widgets}")
-        exceptions = [*self.context.exceptions_children, *self.context_root.exceptions_self]
-        # break the reference cycles through the tree (see _teardown_component_context);
-        # _closing stays True, making stray setters and event handlers no-ops
-        for context in all_contexts:
-            _teardown_component_context(context)
-        self.context = None
-        self.context_root = None  # type: ignore
-        # the root element, container and root widget keep the widget tree alive, and
-        # widgets reference their elements, whose kwargs hold user callbacks - which
-        # capture use_state setters and therefore this render context
-        self.element = None  # type: ignore
-        self.container = None
-        self.last_root_widget = None
-        self._old_element_ids.clear()
+            # everything below used to run outside the lock: a render() that was
+            # blocked on the lock could then interleave with this teardown and
+            # find self.context None mid-render (AssertionError in
+            # Element.__exit__, seen in production behind a prompt kernel close)
+            if self.container:
+                self.container.close()
+                if isinstance(self.container, widgets.DOMWidget) and self.container.layout is not None:
+                    self.container.layout.close()
+            if self._shared_elements:
+                raise RuntimeError(f"Element not cleaned up: {self._shared_elements}")
+            if self._orphans:
+                orphan_widgets = set([_get_widgets_dict()[k] for k in self._orphans])
+                raise RuntimeError(f"Orphan widgets not cleaned up for widgets: {orphan_widgets}")
+            exceptions = [*self.context.exceptions_children, *self.context_root.exceptions_self]
+            # break the reference cycles through the tree (see _teardown_component_context);
+            # _closing stays True, making stray setters and event handlers no-ops
+            for context in all_contexts:
+                _teardown_component_context(context)
+            self.context = None
+            self.context_root = None  # type: ignore
+            # the root element, container and root widget keep the widget tree alive, and
+            # widgets reference their elements, whose kwargs hold user callbacks - which
+            # capture use_state setters and therefore this render context
+            self.element = None  # type: ignore
+            self.container = None
+            self.last_root_widget = None
+            self._old_element_ids.clear()
         if exceptions:
             raise exceptions[0]
 
@@ -1547,6 +1551,12 @@ def render(self, element: Element, container: widgets.Widget = None):
             self._lock_thread = threading.current_thread()
             if was_locked:
                 logger.info("Mutex released, continuing render phase")
+            if self._closing or self.context is None:
+                # close() won the race for the lock (a disconnect can close the
+                # kernel while an update was waiting to render): the tree is
+                # torn down, there is nothing to render into anymore
+                logger.info("Render requested on a closing/closed render context, ignoring")
+                return container
             prev_rc = getattr(local, "rc", None)
             try:
                 local.rc = self

reacton/core_test.py

@@ -2963,6 +2963,66 @@ def test_close_when_overridden():
     rc.close()
 
 
+def test_render_after_close_is_noop():
+    # A disconnect can close the kernel while an update is waiting on the render
+    # lock: the late render used to proceed into the torn-down tree and hit
+    # "assert rc.context is not None" in Element.__exit__ (seen in production).
+    # A render on a closed/closing context must be a no-op instead.
+    @react.component
+    def App():
+        value, _set_value = react.use_state(1)
+        return w.Label(value=str(value))
+
+    box, rc = react.render(App(), handle_error=False)
+    rc.close()
+    rc.render(App(), box)  # must not raise, must not render
+
+
+def test_close_render_race():
+    # close() in one thread while renders queue behind the lock in another:
+    # neither thread may raise (the close teardown must not interleave with a
+    # render that was blocked on the lock).
+    import threading
+
+    for _ in range(20):
+
+        @react.component
+        def App():
+            value, _set_value = react.use_state(1)
+            return w.Label(value=str(value))
+
+        box, rc = react.render(App(), handle_error=False)
+        el = App()
+        errors: List[BaseException] = []
+        barrier = threading.Barrier(2)
+
+        def do_render(rc=rc, el=el, box=box, errors=errors, barrier=barrier):
+            try:
+                barrier.wait()
+                rc.render(el, box)
+            except BaseException as e:  # noqa
+                errors.append(e)
+
+        def do_close(rc=rc, errors=errors, barrier=barrier):
+            try:
+                barrier.wait()
+                rc.close()
+            except BaseException as e:  # noqa
+                errors.append(e)
+
+        t1 = threading.Thread(target=do_render)
+        t2 = threading.Thread(target=do_close)
+        t1.start()
+        t2.start()
+        t1.join()
+        t2.join()
+        # whichever won the lock, the loser must not blow up; if render won,
+        # close ran after and the context must still close cleanly
+        if not rc._closing:
+            rc.close()
+        assert not errors, errors
+
+
 def test_render_perf_child_only():
     render_child = unittest.mock.Mock()
     render_main = unittest.mock.Mock()