Skip to content

Some busy indicators not firing #1381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jcheng5 opened this issue May 15, 2024 · 3 comments · Fixed by #1388
Closed

Some busy indicators not firing #1381

jcheng5 opened this issue May 15, 2024 · 3 comments · Fixed by #1388

Comments

@jcheng5
Copy link
Collaborator

jcheng5 commented May 15, 2024

With the examples/busy_indicators/app.py app, on Windows, I'm pretty reliably able to get only one out of the four plots to show progress.

https://jam.dev/c/fce934dd-d6de-4514-9ae5-184bbc74684d

I can see from the Python console that Shiny is sending the right messages, but some of them are getting batched up, similar to #1373, except in this case uvloop is not installed. Also, if I use --reload it doesn't happen.

@jcheng5
Copy link
Collaborator Author

jcheng5 commented May 15, 2024

I wonder if we need to get the I/O event loop and the session event loop onto separate threads, like we did in R.

@jcheng5
Copy link
Collaborator Author

jcheng5 commented May 15, 2024

I followed the call to conn.write() with a debugger down to here:
https://github.com/python/cpython/blob/5b88d95cc542cf02303c6fe0e8719a93544decdb/Lib/asyncio/proactor_events.py#L363-L374

When the first branch is taken, the message goes right away. But in the repro case, that's only true for the first progress message; the second message goes to the second branch, and all the remaining ones go to the third branch, and none of those get sent until computation is complete.

jcheng5 added a commit that referenced this issue May 15, 2024
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.
@jcheng5
Copy link
Collaborator Author

jcheng5 commented May 15, 2024

I filed a PR that will fix this. Still doesn't feel awesome that our I/O is tangled up with the app logic like this, I have a feeling this will come back with insert_ui or custom messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant