From 0466a85750b8e1da879683581a92e0d0cf1215f3 Mon Sep 17 00:00:00 2001 From: Maarten Breddels Date: Wed, 30 Sep 2026 10:48:51 +0200 Subject: [PATCH 1/2] Do not let a setter wait for the render lock A thread that holds its own lock while it sets state could deadlock with a render on another thread that needs the same lock: the setter waited for the render lock, and the render waited for the user lock. This happened in the window where the other thread holds the render lock but is not rendering yet. A setter now only tries the render lock. When another thread holds it, the setter leaves its change to that thread, which looks at _rerender_needed again after it released the lock. That is the rule reacton already follows when a setter runs during a render, now also for the short windows around it. The look after the release also covers the lost update that the reorder in #73 fixed, so that reorder is removed: one mechanism instead of two. An explicit render() still waits: it brings a new root element, which only the thread that holds the lock may install. Co-Authored-By: Claude Opus 5.5 (1M context) --- reacton/core.py | 53 +++++++++++++++++++++++--------------- reacton/threads_test.py | 57 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 21 deletions(-) diff --git a/reacton/core.py b/reacton/core.py index 87b9687..0203736 100644 --- a/reacton/core.py +++ b/reacton/core.py @@ -1639,7 +1639,9 @@ def update(self, element: Element): def _possible_rerender(self): if not self._is_rendering and self._batch_counter.current() == 0: - self.render(self.element, self.container) + # do not wait for the render lock: the caller (a setter) may hold a lock that the render + # needs. The thread that holds the render lock renders our change (see render). + self.render(self.element, self.container, wait=False) else: logger.info("No render phase triggered, already rendering") @@ -1672,25 +1674,32 @@ def _discard_aborted_pass(self): self._shared_elements_next = set() self.context = self.context_root - def render(self, element: Element, container: widgets.Widget = None): + def render(self, element: Element, container: widgets.Widget = None, wait: bool = True): # render + consolidate widget = None if container is None: container = self.container - was_locked = False - if self.thread_lock.locked(): - if self._lock_thread == threading.current_thread(): - raise RuntimeError("Recursive render detected (avoided deadlock), current thread: %r" % threading.current_thread()) - logger.info( - "Render phase still in progress, waiting for mutex to release (locked obtained by %r, we are in thread %r)", - self._lock_thread, - threading.current_thread(), - ) - was_locked = True - with self.thread_lock: - self._lock_thread = threading.current_thread() - if was_locked: + locked = False + try: + # acquire inside the try, in one statement with the assignment: an exception raised + # on the next line (an interrupt, a cancel from a trace function) still releases the lock + locked = self.thread_lock.acquire(blocking=False) + if not locked: + if self._lock_thread == threading.current_thread(): + raise RuntimeError("Recursive render detected (avoided deadlock), current thread: %r" % threading.current_thread()) + if not wait: + # the thread that holds the lock looks at _rerender_needed after it released the + # lock (at the end of this method), and renders our change + logger.info("Render phase in progress in thread %r, leaving the render to it", self._lock_thread) + return container + logger.info( + "Render phase still in progress, waiting for mutex to release (locked obtained by %r, we are in thread %r)", + self._lock_thread, + threading.current_thread(), + ) + locked = self.thread_lock.acquire() logger.info("Mutex released, continuing render phase") + 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 @@ -1825,13 +1834,7 @@ def format(reason: RerenderReason): # an exception bubbled up during reconsolidate break - # A setter on another thread sets _rerender_needed and then reads _is_rendering: - # while that is True, it leaves the render to us. So clear _is_rendering before - # the last look at _rerender_needed: either we see its change here, or the setter - # sees that we are done and renders the change itself. - self._is_rendering = False if self._rerender_needed: - self._is_rendering = True logger.info("Need rerender after reconsolidation: %r", self._rerender_needed_reasons[-1]) stable = False else: @@ -1869,6 +1872,9 @@ def format(reason: RerenderReason): # while a *different* thread holds the lock (false "Recursive render") self._lock_thread = None assert self.context is self.context_root + finally: + if locked: + self.thread_lock.release() exceptions = [*self.context.exceptions_children, *self.context_root.exceptions_self] if exceptions: @@ -1890,6 +1896,11 @@ def format(reason: RerenderReason): return self.render(w.HTML(value="
" + value + "
", layout=w.Layout(overflow="auto")), self.container) else: raise exc + # A setter on another thread 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 its + # change here, or the setter got the lock and renders the change itself. + if self._rerender_needed: + self._possible_rerender() return widget def _render(self, element: Element, default_key: str, parent_key: str): diff --git a/reacton/threads_test.py b/reacton/threads_test.py index b1a8c98..80995da 100644 --- a/reacton/threads_test.py +++ b/reacton/threads_test.py @@ -1,3 +1,4 @@ +import logging import threading import reacton @@ -69,3 +70,59 @@ def report_progress(): assert not other.is_alive() assert box.children[0].description == f"1 {n}" rc.close() + + +def test_setter_that_holds_a_user_lock_does_not_deadlock_with_a_render(): + # Another thread holds its own lock while it sets state, and the render on this thread takes that + # lock in an effect. If the setter waits for the render lock (held by this thread), this thread + # waits for the user lock (held by the setter): a deadlock. The effect uses a timeout to break it. + user_lock = threading.RLock() + deadlocked = [] + setters = {} + + @reacton.component + def Test(): + a, setters["a"] = reacton.use_state(0) + b, setters["b"] = reacton.use_state(0) + + def effect(): + if user_lock.acquire(timeout=2): + user_lock.release() + else: + deadlocked.append(True) + + reacton.use_effect(effect, [a]) + return w.Button(description=f"{a} {b}") + + box, rc = reacton.render(Test(), handle_error=False) + + def set_b_holding_user_lock(): + with user_lock: + setters["b"](1) + + other = threading.Thread(target=set_b_holding_user_lock) + + class SetBAtRenderStart(logging.Filter): + # "Render phase: " is logged after the render took the render lock, but before it renders + def filter(self, record): + if str(record.msg).startswith("Render phase: ") and other.ident is None: + other.start() + # returns at once when the setter does not wait for the render lock + other.join(0.5) + return True + + logger = logging.getLogger("reacton") + level = logger.level + set_b = SetBAtRenderStart() + logger.setLevel(logging.INFO) + logger.addFilter(set_b) + try: + setters["a"](1) + finally: + logger.removeFilter(set_b) + logger.setLevel(level) + other.join(TIMEOUT) + assert not other.is_alive() + assert not deadlocked, "the setter waited for the render lock while it held the user lock" + assert box.children[0].description == "1 1" + rc.close() From 5013b1daba5ba6ba2b52a62a98c41f4d6c5442cf Mon Sep 17 00:00:00 2001 From: Maarten Breddels Date: Wed, 30 Sep 2026 11:08:55 +0200 Subject: [PATCH 2/2] Do not let update(), render() or force_update() wait for the render lock They could deadlock in the same way as a setter: a thread that holds its own lock asks for a render, waits for the render lock, while the render on the thread that holds it waits for that same lock. They waited because they bring a new root element, and only the thread that holds the lock may install it. Now every render request writes its element to _element_next, marks _rerender_needed and only tries the lock. The thread that holds the lock takes _element_next at the start of each pass, and looks at _rerender_needed again after it released the lock. So render() needs no wait parameter anymore, and only close() still waits for the render lock: it must not tear down the tree during a render. Co-Authored-By: Claude Opus 5.5 (1M context) --- reacton/core.py | 54 +++++++++++++++++++++-------------------- reacton/threads_test.py | 45 +++++++++++++++++++++------------- 2 files changed, 56 insertions(+), 43 deletions(-) diff --git a/reacton/core.py b/reacton/core.py index 0203736..3a37526 100644 --- a/reacton/core.py +++ b/reacton/core.py @@ -1344,6 +1344,9 @@ def _keep_keyed_child(self, context: ComponentContext, el: Element, key: str) -> def __init__(self, element: Element, container: widgets.Widget = None, children_trait="children", handle_error: bool = True, initial_state=None): self.element = element + # the last requested root element: the thread that holds the render lock copies it to + # self.element at the start of each pass (see render) + self._element_next = element self.container = container self.children_next_trait = children_trait self.first_render = True @@ -1454,6 +1457,7 @@ def collect(context: ComponentContext): # 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._element_next = None # type: ignore self.container = None self.last_root_widget = None self._old_element_ids.clear() @@ -1607,7 +1611,7 @@ def force_update(self): # a forced update re-walks the whole tree, no subtree skipping self._walk_all = True if not self._is_rendering: - self.render(self.element, self.container) + self.render() def use_effect(self, effect: EffectCallable, dependencies=None): assert self.context is not None @@ -1631,17 +1635,15 @@ def use_effect(self, effect: EffectCallable, dependencies=None): def update(self, element: Element): self._walk_all = True if self._is_rendering: - self.element = element + self._element_next = element self._rerender_needed_reasons.append(RerenderReason(reason="root element changed")) self._rerender_needed = True else: - self.render(element, self.container) + self.render(element) def _possible_rerender(self): if not self._is_rendering and self._batch_counter.current() == 0: - # do not wait for the render lock: the caller (a setter) may hold a lock that the render - # needs. The thread that holds the render lock renders our change (see render). - self.render(self.element, self.container, wait=False) + self.render() else: logger.info("No render phase triggered, already rendering") @@ -1674,31 +1676,30 @@ def _discard_aborted_pass(self): self._shared_elements_next = set() self.context = self.context_root - def render(self, element: Element, container: widgets.Widget = None, wait: bool = True): + def render(self, element: Optional[Element] = None, container: widgets.Widget = None): # render + consolidate + # 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). + if self._lock_thread == threading.current_thread(): + raise RuntimeError("Recursive render detected (avoided deadlock), current thread: %r" % threading.current_thread()) widget = None if container is None: container = self.container + if element is not None: + self._element_next = element + if self._rerender_needed is False: + self._rerender_needed_reasons.append(RerenderReason(reason="render requested")) + self._rerender_needed = True locked = False try: # acquire inside the try, in one statement with the assignment: an exception raised # on the next line (an interrupt, a cancel from a trace function) still releases the lock locked = self.thread_lock.acquire(blocking=False) if not locked: - if self._lock_thread == threading.current_thread(): - raise RuntimeError("Recursive render detected (avoided deadlock), current thread: %r" % threading.current_thread()) - if not wait: - # the thread that holds the lock looks at _rerender_needed after it released the - # lock (at the end of this method), and renders our change - logger.info("Render phase in progress in thread %r, leaving the render to it", self._lock_thread) - return container - logger.info( - "Render phase still in progress, waiting for mutex to release (locked obtained by %r, we are in thread %r)", - self._lock_thread, - threading.current_thread(), - ) - locked = self.thread_lock.acquire() - logger.info("Mutex released, continuing render phase") + logger.info("Render phase in progress in thread %r, leaving the render to it", self._lock_thread) + return container 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 @@ -1711,11 +1712,11 @@ def render(self, element: Element, container: widgets.Widget = None, wait: bool in_render_phase = True try: local.rc = self - self.element = element - del element main_render_phase = not self._is_rendering 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.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 self._is_rendering = True @@ -1766,6 +1767,7 @@ def format(reason: RerenderReason): raise RuntimeError(msg) logger.info("Entering nested render phase: %r", self._rerender_needed_reasons[-1]) self._rerender_needed = False + self.element = self._element_next self._shared_elements_next = set() self.context.exception_handler = False self.context.exceptions_children = [] @@ -1896,9 +1898,9 @@ def format(reason: RerenderReason): return self.render(w.HTML(value="
" + value + "
", layout=w.Layout(overflow="auto")), self.container) else: raise exc - # A setter on another thread 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 its - # change here, or the setter got the lock and renders the change itself. + # 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 diff --git a/reacton/threads_test.py b/reacton/threads_test.py index 80995da..ebfeaf3 100644 --- a/reacton/threads_test.py +++ b/reacton/threads_test.py @@ -1,6 +1,8 @@ import logging import threading +import pytest + import reacton import reacton.ipywidgets as w @@ -72,16 +74,18 @@ def report_progress(): rc.close() -def test_setter_that_holds_a_user_lock_does_not_deadlock_with_a_render(): - # Another thread holds its own lock while it sets state, and the render on this thread takes that - # lock in an effect. If the setter waits for the render lock (held by this thread), this thread - # waits for the user lock (held by the setter): a deadlock. The effect uses a timeout to break it. +@pytest.mark.parametrize("request_render", ["set_state", "update", "render", "force_update"]) +def test_render_request_while_holding_a_user_lock_does_not_deadlock(request_render): + # Another thread holds its own lock while it asks for a render (a state change, update(), render() + # or force_update()), and the render on this thread takes that lock in an effect. If the other + # thread waits for the render lock (held by this thread), this thread waits for the user lock + # (held by the other thread): a deadlock. The effect uses a timeout to break it. user_lock = threading.RLock() deadlocked = [] setters = {} @reacton.component - def Test(): + def Test(label=""): a, setters["a"] = reacton.use_state(0) b, setters["b"] = reacton.use_state(0) @@ -92,37 +96,44 @@ def effect(): deadlocked.append(True) reacton.use_effect(effect, [a]) - return w.Button(description=f"{a} {b}") + return w.Button(description=f"{label}{a} {b}") box, rc = reacton.render(Test(), handle_error=False) - - def set_b_holding_user_lock(): + requests = { + "set_state": lambda: setters["b"](1), + "update": lambda: rc.update(Test(label="new ")), + "render": lambda: rc.render(Test(label="new ")), + "force_update": lambda: rc.force_update(), + } + expected = {"set_state": "1 1", "update": "new 1 0", "render": "new 1 0", "force_update": "1 0"} + + def request_holding_user_lock(): with user_lock: - setters["b"](1) + requests[request_render]() - other = threading.Thread(target=set_b_holding_user_lock) + other = threading.Thread(target=request_holding_user_lock) - class SetBAtRenderStart(logging.Filter): + class RequestAtRenderStart(logging.Filter): # "Render phase: " is logged after the render took the render lock, but before it renders def filter(self, record): if str(record.msg).startswith("Render phase: ") and other.ident is None: other.start() - # returns at once when the setter does not wait for the render lock + # returns at once when the other thread does not wait for the render lock other.join(0.5) return True logger = logging.getLogger("reacton") level = logger.level - set_b = SetBAtRenderStart() + request = RequestAtRenderStart() logger.setLevel(logging.INFO) - logger.addFilter(set_b) + logger.addFilter(request) try: setters["a"](1) finally: - logger.removeFilter(set_b) + logger.removeFilter(request) logger.setLevel(level) other.join(TIMEOUT) assert not other.is_alive() - assert not deadlocked, "the setter waited for the render lock while it held the user lock" - assert box.children[0].description == "1 1" + assert not deadlocked, "the other thread waited for the render lock while it held the user lock" + assert box.children[0].description == expected[request_render] rc.close()