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()