diff --git a/reacton/core.py b/reacton/core.py index dfb881c..c714691 100644 --- a/reacton/core.py +++ b/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 diff --git a/reacton/core_test.py b/reacton/core_test.py index 9d6b0e6..0ae6363 100644 --- a/reacton/core_test.py +++ b/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()