Flush cycle refactor

by Joe Cheng
October 5, 2017

In preparation for making the flush cycle async-aware, I've done some refactoring to make the flush cycle easier to reason about even for synchronous code.

Let's define the flush cycle, which wasn't a formal concept until this work started. It is akin to the JavaScript event loop, in that it's something that happens over and over again as the app is executing, and never stops until the app terminates.

The flush cycle has several steps.

1. "Something" happens to make arbitrary code execute. This can be any of:
   1. Message received from browser that causes a session's input values to change
   2. An invalidateLater expires, causing a reactive expression or observer to be invalidated
   3. More esoteric cases, like a file is downloaded and the downloadHandler runs.
2. There may be reactive observers that were invalidated and waiting to run ("pending execution") at this point. If that's so, then flushReact.
3. Running flushReact may have caused sessions to have updated output that needs to be sent to the client.
   1. Run session$onFlush callbacks, if any.
   2. Flush output/errors/input-msgs to the client. If we have sent progressKeys to the client, then do this even if output/errors/input-msgs is empty, as this alerts the client that anything that was previously marked in progress is now no longer in progress. If none of output/error/input-msg/progress-key exist, then skip this step entirely.
   3. Run session$onFlushed callbacks, if any.

Previous to this work, the flush cycle didn't exist in one place. Instead, there were several places that flushReact + flushAllSessions could be invoked.

1. serviceApp, which is called in a loop by runApp(). In this case, flushReact/flushAllSessions is only invoked after a scheduled task is executed.
2. In the websocket onMessage handler, i.e. after some data arrived from the client
3. Some other random places like when the websocket closed (in case something reactive happened in session$onSessionEnded or whatever) or after a file download completed. Basically anywhere arbitrary user code is run, we have to call flushReact/flushAllSessions just in case something might have happened. We only discovered this over time.

I have never felt comfortable with this state of affairs. The goal of this work is to centralize the flush cycle within the main event loop. The previous call sites of flushReact/flushAllSessions should go away or be changed to session$requestFlush(). With the actual flushing all being performed inside the main event loop, it becomes clearer how to change the way it behaves without breaking anything.

Previously, flushAllSessions would invoke flushOutput on literally all active sessions. So receiving a websocket message for Session B would cause session$onFlushed to be invoked for Session A. This turns out not to be harmful in most cases because the default for onFlush/onFlushed is once=TRUE and they are usually called from observers, so in that case you don't see a lot of spurious callbacks happening. But it still seems wrong to me. During the refactor, I tried to make it so flushAllSessions only actually flushed sessions that needed it, so I changed the name to flushPendingSessions and sessions need to call session$requestFlush() to be added to the list of sessions that will be output-flushed next time.

TODO: talk about manageHiddenOutputs and why we no longer need to call it from the flush cycle