Do not lose or reject state changes from other threads during a render

reacton/core.py

@@ -1355,6 +1355,9 @@ def __init__(self, element: Element, container: widgets.Widget = None, children_
         self.last_root_widget: widgets.Widget = None
         self._is_rendering = False
         self._rerender_needed = False
+        self._state_set_by_other_thread = False
+        # used for tests only: called when a render loop is done, right after its last look at _rerender_needed
+        self._on_render_loop_done: Optional[Callable[[], None]] = None
         # the reasons are only read for the "too many renders" error message, and a reason
         # holds the previous and next state value: keeping all of them kept every old state
         # value alive until close(). REACTON_RERENDER_REASONS keeps more, for debugging.
@@ -1574,6 +1577,9 @@ def set_(value):
                 # TODO: enable
                 context.needs_render = True
                 _mark_needs_render_ancestors(context)
+                if self._lock_thread is not threading.current_thread():
+                    # not part of the render loop of the thread that renders (see render)
+                    self._state_set_by_other_thread = True
                 if self._rerender_needed is False:
                     if DEBUG:
                         trigger_stack = traceback.format_stack()
@@ -1729,6 +1735,11 @@ def render(self, element: Element, container: widgets.Widget = None):
                         # 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
+                                self._state_set_by_other_thread = False
+                                render_counts = 0
                             if render_counts > 50:
 
                                 def format(reason: RerenderReason):
@@ -1814,11 +1825,19 @@ 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:
                             stable = True
+                            if self._on_render_loop_done is not None:
+                                self._on_render_loop_done()
 
                     self._is_rendering = False
                 self.context = context_prev

reacton/threads_test.py

@@ -0,0 +1,71 @@
+import threading
+
+import reacton
+import reacton.ipywidgets as w
+
+TIMEOUT = 10
+
+
+def test_state_change_from_another_thread_after_the_last_check_is_rendered():
+    # A setter on another thread leaves the render to a running render loop. When it runs just
+    # after the loop took its last look at _rerender_needed, its change must still be rendered.
+    setters = {}
+
+    @reacton.component
+    def Test():
+        a, setters["a"] = reacton.use_state(0)
+        b, setters["b"] = reacton.use_state(0)
+        return w.Button(description=f"{a} {b}")
+
+    box, rc = reacton.render(Test(), handle_error=False)
+    other = threading.Thread(target=lambda: setters["b"](1))
+
+    def set_b_on_other_thread():
+        rc._on_render_loop_done = None
+        other.start()
+        # the setter returns at once when it leaves the render to us,
+        # or it waits for our render lock when it renders the change itself
+        other.join(0.5)
+
+    rc._on_render_loop_done = set_b_on_other_thread
+    setters["a"](1)
+    other.join(TIMEOUT)
+    assert not other.is_alive()
+    assert box.children[0].description == "1 1"
+    rc.close()
+
+
+def test_state_changes_from_another_thread_are_not_a_render_loop():
+    # Another thread changes state once during each render pass (a progress bar, for instance),
+    # more often than the "Too many renders" limit allows for a render loop.
+    n = 60
+    setters = {}
+    remaining = [0]
+    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 remaining[0] > 0:
+            go.release()  # the other thread changes state during this pass
+            assert done.acquire(timeout=TIMEOUT)
+        return w.Button(description=f"{trigger} {progress}")
+
+    box, rc = reacton.render(Test(), handle_error=False)
+
+    def report_progress():
+        for i in range(n):
+            assert go.acquire(timeout=TIMEOUT)
+            setters["progress"](i + 1)
+            remaining[0] -= 1
+            done.release()
+
+    other = threading.Thread(target=report_progress)
+    other.start()
+    remaining[0] = n
+    setters["trigger"](1)
+    other.join(TIMEOUT)
+    assert not other.is_alive()
+    assert box.children[0].description == f"1 {n}"
+    rc.close()