Skip to content
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

Add buffered stdin to accept terminal input whilst running WASM commands #17

Closed
wants to merge 1 commit into from

Conversation

ianthomas23
Copy link
Member

These are changes to support passing stdin from the terminal to a WASM command whilst it is running. It works with the corresponding changes in cockle in jupyterlite/cockle#23.

It uses SharedArrayBuffer to share information from stdin between the main worker (JupyterLite UI thread) and web worker (running the cockle shell), which allows the web worker to perform a synchronous blocking request to obtain the latest stdin. This cannot be performed via standard message passing between main and webworker via comlink as those are async and hence would not be received by the blocking WASM command whilst it is running.

Just before a WASM command is started, buffered stdin is enabled so that stdin messages from the frontend terminal are buffered in the main worker and the first character from stdin is inserted in the SharedArrayBuffer. If the webworker running the WASM command needs stdin it performs a synchronous Atomics.wait on the SharedArrayBuffer which blocks until such a character is available. One item (Int32) of the SharedArrayBuffer is used by the main worker to tell the webworker when a stdin character is available, and another item is used by webworker to tell the main worker that it has taken a character and it may load the next one if available. Stdin is finished when the shell receives an End-Of-Transmission character (ASCII 4) such as via Ctrl-D. Just after the WASM command finishes the buffered stdin is disabled and any characters already buffered are sent to the shell in the conventional way to avoid any data loss.

To try this out locally you need to use

jupyter lite serve --LiteBuildConfig.extra_http_headers=Cross-Origin-Embedder-Policy=require-corp --LiteBuildConfig.extra_http_headers=Cross-Origin-Opener-Policy=same-origin

to enable SharedArrayBuffer.

I am expecting many of the implementation details of the BufferedStdin classes to change when we have to support further functionality in the shell, so consider this just good enough for now.

These changes are really too complicated to be in this repo. Instead they and the webworker code should probably be move to cockle so keep this repo as simple as possible, relating only to integration with JupyterLite.

@ianthomas23 ianthomas23 added the enhancement New feature or request label Jul 22, 2024
@ianthomas23
Copy link
Member Author

I have been using wc to test this. Without any arguments it takes its input from stdin until Ctrl-D is received, when it calculates and shows the number of lines, words and characters. I have not included a screencast of this as it is really dull given that those stdin characters are purposefully not echoed back to stdout.

@ianthomas23
Copy link
Member Author

@jtpio I presume that this will break the github pages deployment as some changes will be required to enable SharedArrayBuffer.

@jtpio
Copy link
Member

jtpio commented Jul 22, 2024

@jtpio I presume that this will break the github pages deployment as some changes will be required to enable SharedArrayBuffer.

Yes. The documentation is still lacking some information about how to enable the SharedArrayBuffer on GitHub Pages: jupyterlite/jupyterlite#1409

Using something like https://github.com/WebReflection/mini-coi

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an alternative, maybe it could be interesting to consider using coincident as an abstraction layer for the communication between the main thread and the worker?

This is what we use for the kernels. jupyterlite/pyodide-kernel#126 reworks the logic a bit to use coincident when SharedArrayBuffer are available, and comlink otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes to coincident eventually. I have a branch predating this work that replaces use of comlink with coincident and it seemed to work OK but with quite a lot of warnings that I'll need to work through.

The link to your coincident-comlink fallback code is useful, thanks. We could do similar here as currently one can still have a reasonable terminal experience without SharedArrayBuffer, but when we start to add some of the more useful interactive commands like more, less and vim it might be preferable to give users a hard fail if SharedArrayBuffer isn't available rather than let them experience a not-very-interactive terminal and think that that is as good as it gets.

Copy link
Member

@jtpio jtpio Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a branch predating this work that replaces use of comlink with coincident and it seemed to work OK but with quite a lot of warnings that I'll need to work through.

Based on the discussion in jupyterlite/pyodide-kernel#126 and jupyterlite/jupyterlite#1424, it looks like coincident gives a few errors when SharedArrayBuffer is not available. So maybe we would still need to keep both coincident and comlink if we don't want to force enabling the SharedArrayBuffer.

We could do similar here as currently one can still have a reasonable terminal experience without SharedArrayBuffer, but when we start to add some of the more useful interactive commands like more, less and vim it might be preferable to give users a hard fail if SharedArrayBuffer isn't available rather than let them experience a not-very-interactive terminal and think that that is as good as it gets.

Right, if the terminal can still work (even partially) when SharedArrayBuffer is not available, then maybe it's better for the end user, as they would still be able to use some of the features? Maybe there could be a way to define optional commands based on whether SharedArrayBufffer is available or not.

@ianthomas23
Copy link
Member Author

This is on hold as there is an alternative implementation to investigate that does not use SharedArrayBuffer.

@ianthomas23
Copy link
Member Author

I'm closing this as the WebWorker and SharedArrayBuffer code has been moved to cockle (jupyterlite/cockle#38). Changes will be needed here to use that, but in a separate PR.

@ianthomas23 ianthomas23 deleted the buffered-stdin branch August 13, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants