From 7b9464f430556aaef123cc26499a6d7dbd68a0e3 Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Wed, 15 May 2024 10:07:54 -0700 Subject: [PATCH 1/2] Yield to give "synchronous" writes a chance to complete During a synchronous phase of the event loop, we can build up network messages that need to get to the client. Because writes are naturally async, we do some weird stuff to semi-execute them which usually but doesn't always succeed. In the case where the write doesn't succeed, they're held at least until the next time the current task yields; before this commit, that could be a long time indeed, if the app is written synchronously (as about 100% of Shiny apps are). This commit yields to the event loop after invalidating the session, which gives the writes a chance to complete. This should take care of the observed problem of progress messages not making it (issue #1381). It also yields after each successful @effect, in case insert_ui, custom messages, etc. were sent during the @effect, and also so outputs' automatic progress messages can propagate to the client. --- shiny/reactive/_reactives.py | 5 +++++ shiny/session/_session.py | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/shiny/reactive/_reactives.py b/shiny/reactive/_reactives.py index 1503b42b2..c8bf7e920 100644 --- a/shiny/reactive/_reactives.py +++ b/shiny/reactive/_reactives.py @@ -15,6 +15,7 @@ "event", ) +import asyncio import functools import traceback import warnings @@ -575,6 +576,10 @@ async def _run(self) -> None: try: with ctx(): await self._fn() + + # Yield so that messages can be sent to the client if necessary. + # https://github.com/posit-dev/py-shiny/issues/1381 + await asyncio.sleep(0) except SilentException: # It's OK for SilentException to cause an Effect to stop running pass diff --git a/shiny/session/_session.py b/shiny/session/_session.py index ff3880d76..92f27374b 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -4,6 +4,7 @@ __all__ = ("Session", "Inputs", "Outputs") +import asyncio import contextlib import dataclasses import enum @@ -653,6 +654,12 @@ def verify_state(expected_state: ConnectionState) -> None: f"Unrecognized method {message_obj['method']}" ) + # Progress messages (of the "{binding: {id: xxx}}"" variety) may + # have queued up at this point; let them drain before we send + # the next message. + # https://github.com/posit-dev/py-shiny/issues/1381 + await asyncio.sleep(0) + self._request_flush() await flush() From 372acaf79696e205eb3a0c3aae075bbd67db0f8b Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Wed, 15 May 2024 20:41:31 -0700 Subject: [PATCH 2/2] Fix hanging unit test --- tests/pytest/mocktime.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/pytest/mocktime.py b/tests/pytest/mocktime.py index 6073475ef..4c9026777 100644 --- a/tests/pytest/mocktime.py +++ b/tests/pytest/mocktime.py @@ -67,6 +67,16 @@ async def _sleep(self, delay: float) -> None: self._i += 1 # Oldest first self._sleepers.sort() + + # This is necessary for some tests that call logic that internally awaits on + # asyncio.sleep(0). Without this, they hang. + # + # Another potential workaround would've been to check if delay <= 0 and just + # return. But this solution has the nice property of actually yielding control + # (I think...!), which is the whole point of asyncio.sleep(0). + if not self._is_advancing: + await self.advance_time(0) + await wake.wait()