From 58de7a7be224ab010523bb9836609e04ecc6144a Mon Sep 17 00:00:00 2001 From: Maarten Breddels Date: Wed, 30 Sep 2026 11:41:25 +0200 Subject: [PATCH] Bound the render loop for changes from other threads, and do not nest renders Since #73, a change from another thread started the "Too many renders" count again. A thread that changes state during every render pass (a loop without a pause) then kept the rendering thread busy for as long as it ran: that thread's caller did not return, and close() waited for it. In solara this is often the kernel thread, so clicks were not handled, and the teardown, which waits for the message in flight, could wait for a task that only stops when the kernel closes. A change from another thread now raises the limit from 50 to 100 passes, once, instead of starting the count again. Past it, "Too many renders" says that another thread changes state during every pass. The limit for a render's own loop stays 50: the mark is cleared when a render starts. The look after the release rendered again by calling render() recursively. When a change lands after the last look many times in a row, that nested without end (a RecursionError); render() now loops. Co-Authored-By: Claude Opus 5.5 (1M context) --- reacton/core.py | 43 +++++++++++++------- reacton/threads_test.py | 87 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 15 deletions(-) diff --git a/reacton/core.py b/reacton/core.py index 0394043..7fafa02 100644 --- a/reacton/core.py +++ b/reacton/core.py @@ -1689,7 +1689,19 @@ def render(self, element: Optional[Element] = None, container: widgets.Widget = # We never wait for the render lock: the caller may hold a lock that the render needs (a # deadlock). So we first mark the request, then only try the lock. When another thread holds # it, that thread renders the request: it takes _element_next at the start of each pass, and - # looks at _rerender_needed again after it released the lock (at the end of this method). + # looks at _rerender_needed again after it released the lock (below). + widget, rendered = self._render_once(element, container) + # A request from another thread (a setter, update(), render()) sets _rerender_needed, and leaves + # the render to us while we render or hold the render lock. We released the lock, and look again + # now: either we see the request here, or that thread got the lock and renders it itself. A loop, + # not a recursion: this can repeat many times in a row while another thread changes state. + while rendered and self._rerender_needed and not self._is_rendering and self._batch_counter.current() == 0: + widget, rendered = self._render_once(None, container) + return widget + + def _render_once(self, element: Optional[Element], container: widgets.Widget) -> Tuple[Any, bool]: + # returns the root widget, and whether we rendered (False: another thread holds the render + # lock and renders our request, or we are closed, or we rendered an error message) if self._lock_thread == threading.current_thread(): raise RuntimeError("Recursive render detected (avoided deadlock), current thread: %r" % threading.current_thread()) widget = None @@ -1707,14 +1719,14 @@ def render(self, element: Optional[Element] = None, container: widgets.Widget = locked = self.thread_lock.acquire(blocking=False) if not locked: logger.info("Render phase in progress in thread %r, leaving the render to it", self._lock_thread) - return container + return container, False self._lock_thread = threading.current_thread() 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 + return container, False prev_rc = getattr(local, "rc", None) # an exception that escapes while this is True aborted a render pass (see the except below) in_render_phase = True @@ -1724,6 +1736,7 @@ def render(self, element: Optional[Element] = None, container: widgets.Widget = render_count = self.render_count # make a copy # clear before taking the element: a request that comes in between is seen by the loop self._rerender_needed = False + self._state_set_by_other_thread = False self.element = self._element_next logger.info("Render phase: %r %r of %r", self.render_count, "main" if main_render_phase else "(nested)", self.element) self.render_count += 1 @@ -1749,16 +1762,18 @@ def render(self, element: Optional[Element] = None, container: widgets.Widget = if main_render_phase: stable = False render_counts = 0 + render_limit = 50 while not stable and not self.context_root.exceptions_children: # we started the rendering loop (main_render_phase is True), so we keep going # but if an exception bubbled up, we should stop while self._rerender_needed and not self.context_root.exceptions_children: if self._state_set_by_other_thread: - # another thread changed state during the last pass (a progress update - # for instance): that is not a render loop, so start counting again + # another thread changed state during a pass (a progress update for + # instance): that is not a render loop of our own, so allow more passes. + # But not without end: our caller would not return, and close() waits. self._state_set_by_other_thread = False - render_counts = 0 - if render_counts > 50: + render_limit = 100 + if render_counts > render_limit: def format(reason: RerenderReason): f = f"Reason: {reason.reason}\nValue changed from {reason.prev_value} to {reason.next_value}\n" @@ -1768,7 +1783,10 @@ def format(reason: RerenderReason): f += f"Triggered at: {''.join(reason.trigger_stack)}\n" return f - msg = f"Too many renders triggered, your render loop does not stop\nLast reason: {format(self._rerender_needed_reasons[-1])}\n" + msg = "Too many renders triggered, your render loop does not stop\n" + if render_limit > 50: + msg += "Another thread changes state during every render pass: add a pause between its changes\n" + msg += f"Last reason: {format(self._rerender_needed_reasons[-1])}\n" if len(self._rerender_needed_reasons) >= 2: previous = reversed(list(self._rerender_needed_reasons)[:-1]) msg += f"Previous reasons: {''.join(format(reason) for reason in previous)}\n" @@ -1903,15 +1921,10 @@ def format(reason: RerenderReason): value = html.escape(error) from . import ipywidgets as w - return self.render(w.HTML(value="
" + value + "
", layout=w.Layout(overflow="auto")), self.container) + return self.render(w.HTML(value="
" + value + "
", layout=w.Layout(overflow="auto")), self.container), False else: raise exc - # A request from another thread (a setter, update(), render()) sets _rerender_needed, and leaves - # the render to us while we render or hold the render lock. We released the lock above, and look - # again now: either we see the request here, or that thread got the lock and renders it itself. - if self._rerender_needed: - self._possible_rerender() - return widget + return widget, True def _render(self, element: Element, default_key: str, parent_key: str): if not isinstance(element, Element): diff --git a/reacton/threads_test.py b/reacton/threads_test.py index c0c378e..3204759 100644 --- a/reacton/threads_test.py +++ b/reacton/threads_test.py @@ -195,3 +195,90 @@ def Test(): assert not thread.is_alive(), "close() from a cleanup during close() hangs" assert not errors, errors rc.close() + + +def test_render_loop_for_another_thread_is_bounded(): + # Another thread changes state during every render pass, and does not stop (a loop without a + # pause). The thread that renders must not keep rendering those changes forever: it would not + # return, and close() waits for it. It stops with "Too many renders", which names the cause. + setters = {} + stop = threading.Event() + go, done = threading.Semaphore(0), threading.Semaphore(0) + + @reacton.component + def Test(): + trigger, setters["trigger"] = reacton.use_state(0) + progress, setters["progress"] = reacton.use_state(0) + if trigger and not stop.is_set(): + go.release() # the other thread changes state during this pass + done.acquire(timeout=TIMEOUT) + return w.Button(description=f"{trigger} {progress}") + + box, rc = reacton.render(Test(), handle_error=False) + + def report_progress(): + i = 0 + while not stop.is_set(): + if go.acquire(timeout=0.1): + i += 1 + setters["progress"](i) + done.release() + + other = threading.Thread(target=report_progress, daemon=True) + other.start() + thread, errors = _run_in_thread(lambda: setters["trigger"](1)) + returned_by_itself = not thread.is_alive() + stop.set() + thread.join(TIMEOUT) + other.join(TIMEOUT) + assert returned_by_itself, "the render kept rendering the changes of another thread" + assert len(errors) == 1 and "another thread" in str(errors[0]).lower(), errors + rc.close() + + +def test_many_changes_after_the_last_look_in_a_row_do_not_recurse(): + # A change after the last look of the render loop is rendered after the render lock is released + # (see the lost-update test above). When that happens many times in a row, the renders must not + # nest (a RecursionError). + setters = {} + + @reacton.component + def Test(): + a, setters["a"] = reacton.use_state(0) + return w.Button(description=f"{a}") + + box, rc = reacton.render(Test(), handle_error=False) + n = 2000 + + def change_after_the_last_look(): + a = int(box.children[0].description) + if 0 < a < n: + setters["a"](a + 1) # a render is running: this only marks _rerender_needed + + rc._on_render_loop_done = change_after_the_last_look + setters["a"](1) + assert box.children[0].description == f"{n}" + rc.close() + + +def test_own_render_loop_stops_after_about_50_passes(): + # A component that changes its own state on every render: "Too many renders" after about 50 + # passes. The higher limit is only for changes from other threads, not for every render that a + # state change (from outside a render) started. + renders = [] + setters = {} + + @reacton.component + def Test(): + a, setters["a"] = reacton.use_state(0) + renders.append(a) + if a > 0: + setters["a"](a + 1) # never stops + return w.Button(description=f"{a}") + + box, rc = reacton.render(Test(), handle_error=False) + renders.clear() + with pytest.raises(RuntimeError, match="Too many renders"): + setters["a"](1) + assert len(renders) < 60, len(renders) + rc.close()