Skip to content

Commit ecd0aca

Browse files
committed
fix: cancel stale on_load chains via handler-declared supersession
A page's on_load chain kept running (and blocked the per-token queue) after the user navigated away (reflex-dev#6593). Instead of teaching the generic EventProcessor about page loads, add a handler-declared supersedes marker (mirroring the background-task marker): enqueuing a superseding handler cancels the previous unfinished chain for the same handler and client token. on_load_internal is the first handler to opt in. Also tighten the cancellation lifecycle so a cancelled chain cannot leak work: futures stay tracked while their task unwinds (late-chained events find their cancelled parent and are dropped), and the dequeue paths treat an already-cleaned future as cancelled instead of executing it. Covered by processor unit tests, a state-level end-to-end test, and a Playwright integration test that navigates away mid-on_load and asserts the stale handler is cancelled while the new page loads promptly.
1 parent 932f20f commit ecd0aca

8 files changed

Lines changed: 523 additions & 7 deletions

File tree

news/6593.bugfix.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Cancel the previous unfinished `on_load` event chain when a newer page navigation arrives for the same client, instead of letting stale page-load work block and outlive the navigation.

packages/reflex-base/src/reflex_base/event/__init__.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ def from_event_type(
185185
)
186186

187187
BACKGROUND_TASK_MARKER = "_reflex_background_task"
188+
SUPERSEDES_MARKER = "_reflex_supersedes"
188189
EVENT_ACTIONS_MARKER = "_rx_event_actions"
189190
UPLOAD_FILES_CLIENT_HANDLER = "uploadFiles"
190191

@@ -442,6 +443,18 @@ def is_background(self) -> bool:
442443
"""
443444
return getattr(self.fn, BACKGROUND_TASK_MARKER, False)
444445

446+
@property
447+
def supersedes(self) -> bool:
448+
"""Whether a newer invocation supersedes an older unfinished one.
449+
450+
When True, enqueuing this handler cancels the previous unfinished
451+
event chain rooted at the same handler for the same client token.
452+
453+
Returns:
454+
True if the event handler is marked as superseding.
455+
"""
456+
return getattr(self.fn, SUPERSEDES_MARKER, False)
457+
445458
def __call__(self, *args: Any, **kwargs: Any) -> "EventSpec":
446459
"""Pass arguments to the handler to get an event spec.
447460
@@ -2778,6 +2791,7 @@ class EventNamespace:
27782791

27792792
# Constants
27802793
BACKGROUND_TASK_MARKER = BACKGROUND_TASK_MARKER
2794+
SUPERSEDES_MARKER = SUPERSEDES_MARKER
27812795
EVENT_ACTIONS_MARKER = EVENT_ACTIONS_MARKER
27822796
_EVENT_FIELDS = _EVENT_FIELDS
27832797
FORM_DATA = FORM_DATA

packages/reflex-base/src/reflex_base/event/processor/event_processor.py

Lines changed: 91 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,11 @@ class EventProcessor:
120120
_futures: dict[str, EventFuture] = dataclasses.field(
121121
default_factory=dict, init=False
122122
)
123+
# Latest-wins tracking for superseding handlers: (event name, token) -> the
124+
# currently active chain root future.
125+
_superseded: dict[tuple[str, str], EventFuture] = dataclasses.field(
126+
default_factory=dict, init=False
127+
)
123128
_token_queues: dict[
124129
str,
125130
collections.deque[tuple[EventQueueEntry, RegisteredEventHandler]],
@@ -311,6 +316,7 @@ async def stop(self, graceful_shutdown_timeout: float | None = None) -> None:
311316
self._queue_task = None
312317
# Discard any pending per-token queue entries.
313318
self._token_queues.clear()
319+
self._superseded.clear()
314320
# Cancel any remaining unresolved futures.
315321
for future in self._futures.values():
316322
if not future.done():
@@ -372,6 +378,8 @@ async def enqueue(
372378
373379
Returns:
374380
An EventFuture that resolves to the result of the associated task.
381+
If the event was chained from an already-cancelled chain, the
382+
returned future is already cancelled and the event is dropped.
375383
"""
376384
if ev_ctx is None:
377385
try:
@@ -395,7 +403,13 @@ async def enqueue(
395403
tracked.add_done_callback(self._on_future_done)
396404
# If this context has a parent, register as a child of the parent's future.
397405
if parent_future is not None:
406+
if parent_future.cancelled():
407+
# The chain this event belongs to was cancelled; the event is
408+
# stillborn and never enters the queue.
409+
tracked.cancel()
410+
return tracked
398411
parent_future.add_child(tracked)
412+
self._supersede_previous(token=token, event=event, tracked=tracked)
399413
await queue.put(EventQueueEntry(event=event, ctx=ev_ctx))
400414
return tracked
401415

@@ -491,14 +505,75 @@ def _try_clean_future(self, future: EventFuture) -> None: # type: ignore[overri
491505
"""
492506
if not future.done():
493507
return
508+
if future.txid in self._tasks:
509+
# The handler task is still running or unwinding; keep the future
510+
# so late-chained events can find their (possibly cancelled) parent.
511+
return
494512
# Not checking future.all_done() to avoid waiting for grandchildren here.
495513
if not all(c.done() for c in future.children):
496514
return
497515
parent = future.parent
498516
self._futures.pop(future.txid, None)
517+
if (
518+
(key := future.supersede_key) is not None
519+
and self._superseded.get(key) is future
520+
and future.all_done()
521+
):
522+
del self._superseded[key]
499523
if parent is not None and parent.txid:
500524
self._try_clean_future(parent)
501525

526+
def _supersede_previous(
527+
self, *, token: str, event: Event, tracked: EventFuture
528+
) -> None:
529+
"""Cancel the previous unfinished chain of a superseding event handler.
530+
531+
Handlers marked with ``supersedes`` (e.g. ``on_load_internal``) use
532+
latest-wins semantics: enqueuing a new invocation cancels the previous
533+
unfinished event chain for the same handler and client token.
534+
535+
Args:
536+
token: The client token associated with the event.
537+
event: The event being enqueued.
538+
tracked: The future of the event being enqueued.
539+
"""
540+
try:
541+
registered = RegistrationContext.get().event_handlers.get(event.name)
542+
except LookupError:
543+
return
544+
if registered is None or not registered.handler.supersedes:
545+
return
546+
key = (event.name, token)
547+
previous = self._superseded.get(key)
548+
if (
549+
previous is not None
550+
and not previous.all_done()
551+
# A chain must not supersede itself (e.g. a handler re-enqueuing
552+
# itself); cancelling an ancestor would cancel ``tracked`` too.
553+
and not self._is_ancestor(previous, tracked)
554+
):
555+
previous.cancel()
556+
self._superseded[key] = tracked
557+
tracked.supersede_key = key
558+
559+
@staticmethod
560+
def _is_ancestor(candidate: EventFuture, future: EventFuture) -> bool:
561+
"""Check whether candidate is an ancestor of future.
562+
563+
Args:
564+
candidate: The potential ancestor future.
565+
future: The future whose parent chain to walk.
566+
567+
Returns:
568+
True if candidate appears in future's parent chain.
569+
"""
570+
parent = future.parent
571+
while parent is not None:
572+
if parent is candidate:
573+
return True
574+
parent = parent.parent
575+
return False
576+
502577
def _on_future_done(self, future: EventFuture) -> None: # type: ignore[override]
503578
"""Callback invoked when an enqueued future completes.
504579
@@ -625,10 +700,13 @@ def _dispatch_next_for_token(self, token: str) -> None:
625700
if not token_queue:
626701
return
627702
entry, registered_handler = token_queue[0]
628-
# Skip cancelled futures.
703+
# Skip cancelled futures. Before a task exists, the only way a future
704+
# can be done (and thus already cleaned up) is cancellation, so a
705+
# missing future also means the entry was cancelled.
629706
future = self._futures.get(entry.ctx.txid)
630-
if future is not None and future.cancelled():
631-
self._try_clean_future(future)
707+
if future is None or future.cancelled():
708+
if future is not None:
709+
self._try_clean_future(future)
632710
token_queue.popleft()
633711
if token_queue:
634712
self._dispatch_next_for_token(token)
@@ -645,10 +723,12 @@ async def _process_queue(self):
645723
with contextlib.suppress(QueueShutDown):
646724
while True:
647725
entry = await queue.get()
648-
if (
649-
future := self._futures.get(entry.ctx.txid)
650-
) is not None and future.cancelled():
651-
self._try_clean_future(future)
726+
# A missing future means the entry was cancelled and already
727+
# cleaned up (see _dispatch_next_for_token).
728+
future = self._futures.get(entry.ctx.txid)
729+
if future is None or future.cancelled():
730+
if future is not None:
731+
self._try_clean_future(future)
652732
queue.task_done()
653733
continue
654734
try:
@@ -757,6 +837,10 @@ def _finish_task(self, task: asyncio.Task):
757837
else:
758838
if future is not None and not future.done():
759839
future.set_result(result)
840+
if future is not None:
841+
# The task is gone; clean up now in case the future resolved
842+
# earlier (e.g. external cancellation) and cleanup was deferred.
843+
self._try_clean_future(future)
760844

761845

762846
__all__ = [

packages/reflex-base/src/reflex_base/event/processor/future.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ class EventFuture(asyncio.Future):
3131
default_factory=asyncio.get_running_loop, repr=False
3232
)
3333

34+
# Key under which this future is registered for latest-wins supersession
35+
# in the EventProcessor, if any.
36+
supersede_key: tuple[str, str] | None = dataclasses.field(default=None, repr=False)
37+
3438
def __post_init__(self) -> None:
3539
"""Call Future.__init__ for the EventFuture."""
3640
super(EventFuture, self).__init__(loop=self.loop)

reflex/state.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
from reflex_base.event import (
3333
BACKGROUND_TASK_MARKER,
3434
EVENT_ACTIONS_MARKER,
35+
SUPERSEDES_MARKER,
3536
Event,
3637
EventHandler,
3738
EventSpec,
@@ -739,6 +740,8 @@ def _copy_fn(fn: Callable) -> Callable:
739740
newfn.__annotations__ = fn.__annotations__
740741
if mark := getattr(fn, BACKGROUND_TASK_MARKER, None):
741742
setattr(newfn, BACKGROUND_TASK_MARKER, mark)
743+
if mark := getattr(fn, SUPERSEDES_MARKER, None):
744+
setattr(newfn, SUPERSEDES_MARKER, mark)
742745
# Preserve event_actions from @rx.event decorator
743746
if event_actions := getattr(fn, EVENT_ACTIONS_MARKER, None):
744747
object.__setattr__(newfn, EVENT_ACTIONS_MARKER, event_actions)
@@ -2464,6 +2467,15 @@ def on_load_internal(self) -> list[Event | EventSpec | event.EventCallback] | No
24642467
]
24652468

24662469

2470+
# A newer navigation supersedes the previous unfinished on_load chain for the
2471+
# same client token, cancelling its stale work (#6593).
2472+
setattr(
2473+
OnLoadInternalState.event_handlers["on_load_internal"].fn,
2474+
SUPERSEDES_MARKER,
2475+
True,
2476+
)
2477+
2478+
24672479
class ComponentState(State, mixin=True):
24682480
"""Base class to allow for the creation of a state instance per component.
24692481

tests/integration/tests_playwright/test_router_query.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313
``on_load``, and updates the router reactively with no further interaction.
1414
* ``rx.redirect(target, replace=True)`` behaves the same but replaces the
1515
current history entry instead of pushing a new one.
16+
* Navigating away while a page's ``on_load`` chain is still running cancels
17+
the stale chain instead of letting it keep running and block the new
18+
page's events (https://github.com/reflex-dev/reflex/issues/6593).
1619
1720
Covers dev and prod modes via ``app_harness_env`` parametrisation.
1821
"""
@@ -30,8 +33,27 @@
3033

3134
def RouterQueryApp():
3235
"""App exercising replaceState, redirect, and redirect(replace=True)."""
36+
import asyncio
37+
3338
import reflex as rx
3439

40+
class SlowLoadState(rx.State):
41+
# Records the lifecycle of the slow on_load handler; "finished;" must
42+
# never be appended when the user navigates away mid-load.
43+
slow_log: str = ""
44+
45+
@rx.event
46+
async def slow_on_load(self):
47+
"""Record start, simulate a slow data load, then record completion.
48+
49+
Yields:
50+
None, to emit the "started;" delta before sleeping.
51+
"""
52+
self.slow_log += "started;"
53+
yield
54+
await asyncio.sleep(2)
55+
self.slow_log += "finished;"
56+
3557
class RouterQueryState(rx.State):
3658
# Incremented by the page on_load handler; proves whether a navigation
3759
# (and thus on_load) actually fired.
@@ -146,10 +168,18 @@ def index():
146168
read_only=True,
147169
id="ping-count",
148170
),
171+
rx.input(value=SlowLoadState.slow_log, read_only=True, id="slow-log"),
172+
)
173+
174+
def slow():
175+
return rx.box(
176+
rx.link("go home", href="/", id="to-index"),
177+
rx.input(value=SlowLoadState.slow_log, read_only=True, id="slow-log"),
149178
)
150179

151180
app = rx.App()
152181
app.add_page(index, route="/", on_load=RouterQueryState.on_load)
182+
app.add_page(slow, route="/slow", on_load=SlowLoadState.slow_on_load)
153183

154184

155185
@pytest.fixture(scope="module")
@@ -275,3 +305,35 @@ def test_redirect_replace_replaces_history_entry(
275305
expect(page).to_have_url(f"{base}/")
276306
expect(page.locator("#name-param")).to_have_value("")
277307
expect(page.locator("#query-str")).to_have_value("")
308+
309+
310+
def test_slow_on_load_completes_without_navigation(
311+
router_query_app: AppHarness, page: Page
312+
):
313+
"""Positive control: staying on the page lets the slow on_load finish."""
314+
base = router_query_app.frontend_url
315+
assert base is not None
316+
page.goto(f"{base.rstrip('/')}/slow")
317+
expect(page.locator("#slow-log")).to_have_value("started;")
318+
expect(page.locator("#slow-log")).to_have_value("started;finished;", timeout=5000)
319+
320+
321+
def test_navigation_cancels_stale_on_load(router_query_app: AppHarness, page: Page):
322+
"""Navigating away mid-load cancels the stale on_load chain (#6593)."""
323+
base = router_query_app.frontend_url
324+
assert base is not None
325+
page.goto(f"{base.rstrip('/')}/slow")
326+
# The slow on_load has started and is now sleeping.
327+
expect(page.locator("#slow-log")).to_have_value("started;")
328+
329+
# Navigate away while the slow handler is still running. The new page's
330+
# on_load runs promptly instead of waiting ~2s behind the stale handler.
331+
page.click("#to-index")
332+
expect(page.locator("#load-count")).to_have_value("1", timeout=1000)
333+
334+
# Wait past the stale handler's sleep, then flush a round-trip; the
335+
# cancelled handler must never have appended "finished;".
336+
page.wait_for_timeout(3000)
337+
page.click("#ping")
338+
expect(page.locator("#ping-count")).to_have_value("1")
339+
expect(page.locator("#slow-log")).to_have_value("started;")

0 commit comments

Comments
 (0)