Multiple Worker based Terminals (#1948)

Multiple Worker based Terminals

pyscript.core/package-lock.json

@@ -1,12 +1,12 @@
 {
     "name": "@pyscript/core",
-    "version": "0.3.19",
+    "version": "0.3.20",
     "lockfileVersion": 3,
     "requires": true,
     "packages": {
         "": {
             "name": "@pyscript/core",
-            "version": "0.3.19",
+            "version": "0.3.20",
             "license": "APACHE-2.0",
             "dependencies": {
                 "@ungap/with-resolvers": "^0.1.0",

pyscript.core/package.json

@@ -1,6 +1,6 @@
 {
     "name": "@pyscript/core",
-    "version": "0.3.19",
+    "version": "0.3.20",
     "type": "module",
     "description": "PyScript",
     "module": "./index.js",

pyscript.core/src/plugins/py-terminal.js

@@ -14,31 +14,73 @@ const notifyAndThrow = (message) => {
     throw new Error(message);
 };
 
+const notParsedYet = (script) => !bootstrapped.has(script);
+
+const onceOnMain = ({ attributes: { worker } }) => !worker;
+
+const bootstrapped = new WeakSet();
+
+let addStyle = true;
+
+// this callback will be serialized as string and it never needs
+// to be invoked multiple times. Each xworker here is bootstrapped
+// only once thanks to the `sync.is_pyterminal()` check.
+const workerReady = ({ interpreter, io, run }, { sync }) => {
+    if (!sync.is_pyterminal()) return;
+
+    // in workers it's always safe to grab the polyscript currentScript
+    run("from polyscript.currentScript import terminal as __terminal__");
+
+    // This part is inevitably duplicated as external scope
+    // can't be reached by workers out of the box.
+    // The detail is that here we use sync though, not readline.
+    const decoder = new TextDecoder();
+    let data = "";
+    const generic = {
+        isatty: true,
+        write(buffer) {
+            data = decoder.decode(buffer);
+            sync.pyterminal_write(data);
+            return buffer.length;
+        },
+    };
+    interpreter.setStdout(generic);
+    interpreter.setStderr(generic);
+    interpreter.setStdin({
+        isatty: true,
+        stdin: () => sync.pyterminal_read(data),
+    });
+
+    io.stderr = (error) => {
+        sync.pyterminal_write(`${error.message || error}\n`);
+    };
+};
+
 const pyTerminal = async () => {
     const terminals = document.querySelectorAll(SELECTOR);
 
-    // no results will look further for runtime nodes
+    const unknown = [].filter.call(terminals, notParsedYet);
-    if (!terminals.length) return;
 
-    // if we arrived this far, let's drop the MutationObserver
+    // no results will look further for runtime nodes
-    // as we only support one terminal per page (right now).
+    if (!unknown.length) return;
-    mo.disconnect();
+    // early flag elements as known to avoid concurrent
+    // MutationObserver invokes of this async handler
+    else unknown.forEach(bootstrapped.add, bootstrapped);
 
     // we currently support only one terminal as in "classic"
-    if (terminals.length > 1) notifyAndThrow("You can use at most 1 terminal.");
+    if ([].filter.call(terminals, onceOnMain).length > 1)
-
+        notifyAndThrow("You can use at most 1 main terminal");
-    const [element] = terminals;
-    // hopefully to be removed in the near future!
-    if (element.matches('script[type="mpy"],mpy-script'))
-        notifyAndThrow("Unsupported terminal.");
 
     // import styles lazily
+    if (addStyle) {
+        addStyle = false;
         document.head.append(
             Object.assign(document.createElement("link"), {
                 rel: "stylesheet",
                 href: new URL("./xterm.css", import.meta.url),
             }),
         );
+    }
 
     // lazy load these only when a valid terminal is found
     const [{ Terminal }, { Readline }, { FitAddon }] = await Promise.all([
@@ -47,6 +89,11 @@ const pyTerminal = async () => {
         import(/* webpackIgnore: true */ "../3rd-party/xterm_addon-fit.js"),
     ]);
 
+    for (const element of unknown) {
+        // hopefully to be removed in the near future!
+        if (element.matches('script[type="mpy"],mpy-script'))
+            notifyAndThrow("Unsupported terminal.");
+
         const readline = new Readline();
 
         // common main thread initialization for both worker
@@ -83,55 +130,26 @@ const pyTerminal = async () => {
 
         // branch logic for the worker
         if (element.hasAttribute("worker")) {
-        // when the remote thread onReady triggers:
-        // setup the interpreter stdout and stderr
-        const workerReady = ({ interpreter, io, run }, { sync }) => {
-            // in workers it's always safe to grab the polyscript currentScript
-            run(
-                "from polyscript.currentScript import terminal as __terminal__",
-            );
-            sync.pyterminal_drop_hooks();
-
-            // This part is inevitably duplicated as external scope
-            // can't be reached by workers out of the box.
-            // The detail is that here we use sync though, not readline.
-            const decoder = new TextDecoder();
-            let data = "";
-            const generic = {
-                isatty: true,
-                write(buffer) {
-                    data = decoder.decode(buffer);
-                    sync.pyterminal_write(data);
-                    return buffer.length;
-                },
-            };
-            interpreter.setStdout(generic);
-            interpreter.setStderr(generic);
-            interpreter.setStdin({
-                isatty: true,
-                stdin: () => sync.pyterminal_read(data),
-            });
-
-            io.stderr = (error) => {
-                sync.pyterminal_write(`${error.message || error}\n`);
-            };
-        };
-
             // add a hook on the main thread to setup all sync helpers
-        // also bootstrapping the XTerm target on main
+            // also bootstrapping the XTerm target on main *BUT* ...
             hooks.main.onWorker.add(function worker(_, xworker) {
+                // ... as multiple workers will add multiple callbacks
+                // be sure no xworker is ever initialized twice!
+                if (bootstrapped.has(xworker)) return;
+                bootstrapped.add(xworker);
+
+                // still cleanup this callback for future scripts/workers
                 hooks.main.onWorker.delete(worker);
+
                 init({
                     disableStdin: false,
                     cursorBlink: true,
                     cursorStyle: "block",
                 });
+
+                xworker.sync.is_pyterminal = () => true;
                 xworker.sync.pyterminal_read = readline.read.bind(readline);
                 xworker.sync.pyterminal_write = readline.write.bind(readline);
-            // allow a worker to drop main thread hooks ASAP
-            xworker.sync.pyterminal_drop_hooks = () => {
-                hooks.worker.onReady.delete(workerReady);
-            };
             });
 
             // setup remote thread JS/Python code for whenever the
@@ -178,6 +196,7 @@ const pyTerminal = async () => {
                 };
             });
         }
+    }
 };
 
 const mo = new MutationObserver(pyTerminal);

pyscript.core/src/sync.js

@@ -1,4 +1,7 @@
 export default {
+    // allow pyterminal checks to bootstrap
+    is_pyterminal: () => false,
+
     /**
      * 'Sleep' for the given number of seconds. Used to implement Python's time.sleep in Worker threads.
      * @param {number} seconds The number of seconds to sleep.

pyscript.core/test/mpy.spec.js

@@ -73,3 +73,8 @@ test('Pyodide + terminal on Worker', async ({ page }) => {
   await page.goto('http://localhost:8080/test/py-terminal-worker.html');
   await page.waitForSelector('html.ok');
 });
+
+test('Pyodide + multiple terminals via Worker', async ({ page }) => {
+  await page.goto('http://localhost:8080/test/py-terminals.html');
+  await page.waitForSelector('html.first.second');
+});

pyscript.core/test/py-terminal-worker.html

@@ -9,6 +9,7 @@
         <style>.xterm { padding: .5rem; }</style>
     </head>
     <body>
-        <py-script src="terminal.py" worker terminal></py-script>
+        <script type="py" src="terminal.py" worker terminal></script>
+        <script type="py" src="terminal.py" worker terminal></script>
     </body>
 </html>

pyscript.core/test/py-terminals.html

@@ -0,0 +1,27 @@
+<!doctype html>
+<html lang="en">
+    <head>
+        <meta charset="UTF-8" />
+        <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+        <title>PyTerminal Main</title>
+        <link rel="stylesheet" href="../dist/core.css">
+        <script type="module" src="../dist/core.js"></script>
+        <style>.xterm { padding: .5rem; }</style>
+    </head>
+    <body>
+        <script type="py" worker terminal>
+            from pyscript import document
+            document.documentElement.classList.add("first")
+
+            import code
+            code.interact()
+        </script>
+        <script type="py" worker terminal>
+            from pyscript import document
+            document.documentElement.classList.add("second")
+
+            import code
+            code.interact()
+        </script>
+    </body>
+</html>

pyscript.core/tests/integration/test_py_terminal.py

@@ -7,6 +7,7 @@ from .support import PageErrors, PyScriptTest, only_worker, skip_worker
 
 
 class TestPyTerminal(PyScriptTest):
+    @skip_worker("We do support multiple worker terminal now")
     def test_multiple_terminals(self):
         """
         Multiple terminals are not currently supported
@@ -19,9 +20,9 @@ class TestPyTerminal(PyScriptTest):
             wait_for_pyscript=False,
             check_js_errors=False,
         )
-        assert self.assert_banner_message("You can use at most 1 terminal")
+        assert self.assert_banner_message("You can use at most 1 main terminal")
 
-        with pytest.raises(PageErrors, match="You can use at most 1 terminal"):
+        with pytest.raises(PageErrors, match="You can use at most 1 main terminal"):
             self.check_js_errors()
 
     # TODO: interactive shell still unclear

pyscript.core/types/sync.d.ts

@@ -1,4 +1,5 @@
 declare namespace _default {
+    function is_pyterminal(): boolean;
     /**
      * 'Sleep' for the given number of seconds. Used to implement Python's time.sleep in Worker threads.
      * @param {number} seconds The number of seconds to sleep.