From f6470dcad56d030cd1528ad26b1d80530addd7ea Mon Sep 17 00:00:00 2001 From: Andrea Giammarchi Date: Wed, 24 Jan 2024 17:33:55 +0100 Subject: [PATCH] Multiple Worker based Terminals (#1948) Multiple Worker based Terminals --- pyscript.core/package-lock.json | 4 +- pyscript.core/package.json | 2 +- pyscript.core/src/plugins/py-terminal.js | 291 ++++++++++-------- pyscript.core/src/sync.js | 3 + pyscript.core/test/mpy.spec.js | 5 + pyscript.core/test/py-terminal-worker.html | 3 +- pyscript.core/test/py-terminals.html | 27 ++ .../tests/integration/test_py_terminal.py | 5 +- pyscript.core/types/sync.d.ts | 1 + 9 files changed, 199 insertions(+), 142 deletions(-) create mode 100644 pyscript.core/test/py-terminals.html diff --git a/pyscript.core/package-lock.json b/pyscript.core/package-lock.json index 16211fcb..0fb24c5b 100644 --- a/pyscript.core/package-lock.json +++ b/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", diff --git a/pyscript.core/package.json b/pyscript.core/package.json index a5deffca..07a92902 100644 --- a/pyscript.core/package.json +++ b/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", diff --git a/pyscript.core/src/plugins/py-terminal.js b/pyscript.core/src/plugins/py-terminal.js index edf9db3c..bd66156b 100644 --- a/pyscript.core/src/plugins/py-terminal.js +++ b/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 - if (!terminals.length) return; + const unknown = [].filter.call(terminals, notParsedYet); - // if we arrived this far, let's drop the MutationObserver - // as we only support one terminal per page (right now). - mo.disconnect(); + // no results will look further for runtime nodes + if (!unknown.length) return; + // 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."); - - const [element] = terminals; - // hopefully to be removed in the near future! - if (element.matches('script[type="mpy"],mpy-script')) - notifyAndThrow("Unsupported terminal."); + if ([].filter.call(terminals, onceOnMain).length > 1) + notifyAndThrow("You can use at most 1 main terminal"); // import styles lazily - document.head.append( - Object.assign(document.createElement("link"), { - rel: "stylesheet", - href: new URL("./xterm.css", import.meta.url), - }), - ); + 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,136 +89,113 @@ const pyTerminal = async () => { import(/* webpackIgnore: true */ "../3rd-party/xterm_addon-fit.js"), ]); - const readline = new Readline(); + for (const element of unknown) { + // hopefully to be removed in the near future! + if (element.matches('script[type="mpy"],mpy-script')) + notifyAndThrow("Unsupported terminal."); - // common main thread initialization for both worker - // or main case, bootstrapping the terminal on its target - const init = (options) => { - let target = element; - const selector = element.getAttribute("target"); - if (selector) { - target = - document.getElementById(selector) || - document.querySelector(selector); - if (!target) throw new Error(`Unknown target ${selector}`); - } else { - target = document.createElement("py-terminal"); - target.style.display = "block"; - element.after(target); - } - const terminal = new Terminal({ - theme: { - background: "#191A19", - foreground: "#F5F2E7", - }, - ...options, - }); - const fitAddon = new FitAddon(); - terminal.loadAddon(fitAddon); - terminal.loadAddon(readline); - terminal.open(target); - fitAddon.fit(); - terminal.focus(); - defineProperty(element, "terminal", { value: terminal }); - return terminal; - }; + const readline = new Readline(); - // 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; + // common main thread initialization for both worker + // or main case, bootstrapping the terminal on its target + const init = (options) => { + let target = element; + const selector = element.getAttribute("target"); + if (selector) { + target = + document.getElementById(selector) || + document.querySelector(selector); + if (!target) throw new Error(`Unknown target ${selector}`); + } else { + target = document.createElement("py-terminal"); + target.style.display = "block"; + element.after(target); + } + const terminal = new Terminal({ + theme: { + background: "#191A19", + foreground: "#F5F2E7", }, - }; - interpreter.setStdout(generic); - interpreter.setStderr(generic); - interpreter.setStdin({ - isatty: true, - stdin: () => sync.pyterminal_read(data), + ...options, }); - - io.stderr = (error) => { - sync.pyterminal_write(`${error.message || error}\n`); - }; + const fitAddon = new FitAddon(); + terminal.loadAddon(fitAddon); + terminal.loadAddon(readline); + terminal.open(target); + fitAddon.fit(); + terminal.focus(); + defineProperty(element, "terminal", { value: terminal }); + return terminal; }; - // add a hook on the main thread to setup all sync helpers - // also bootstrapping the XTerm target on main - hooks.main.onWorker.add(function worker(_, xworker) { - hooks.main.onWorker.delete(worker); - init({ - disableStdin: false, - cursorBlink: true, - cursorStyle: "block", - }); - 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); - }; - }); + // branch logic for the worker + if (element.hasAttribute("worker")) { + // add a hook on the main thread to setup all sync helpers + // 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); - // setup remote thread JS/Python code for whenever the - // worker is ready to become a terminal - hooks.worker.onReady.add(workerReady); - } else { - // in the main case, just bootstrap XTerm without - // allowing any input as that's not possible / awkward - hooks.main.onReady.add(function main({ interpreter, io, run }) { - console.warn("py-terminal is read only on main thread"); - hooks.main.onReady.delete(main); + // still cleanup this callback for future scripts/workers + hooks.main.onWorker.delete(worker); - // on main, it's easy to trash and clean the current terminal - globalThis.__py_terminal__ = init({ - disableStdin: true, - cursorBlink: false, - cursorStyle: "underline", - }); - run("from js import __py_terminal__ as __terminal__"); - delete globalThis.__py_terminal__; + init({ + disableStdin: false, + cursorBlink: true, + cursorStyle: "block", + }); - // 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 readline here, not sync. - const decoder = new TextDecoder(); - let data = ""; - const generic = { - isatty: true, - write(buffer) { - data = decoder.decode(buffer); - readline.write(data); - return buffer.length; - }, - }; - interpreter.setStdout(generic); - interpreter.setStderr(generic); - interpreter.setStdin({ - isatty: true, - stdin: () => readline.read(data), + xworker.sync.is_pyterminal = () => true; + xworker.sync.pyterminal_read = readline.read.bind(readline); + xworker.sync.pyterminal_write = readline.write.bind(readline); }); - io.stderr = (error) => { - readline.write(`${error.message || error}\n`); - }; - }); + // setup remote thread JS/Python code for whenever the + // worker is ready to become a terminal + hooks.worker.onReady.add(workerReady); + } else { + // in the main case, just bootstrap XTerm without + // allowing any input as that's not possible / awkward + hooks.main.onReady.add(function main({ interpreter, io, run }) { + console.warn("py-terminal is read only on main thread"); + hooks.main.onReady.delete(main); + + // on main, it's easy to trash and clean the current terminal + globalThis.__py_terminal__ = init({ + disableStdin: true, + cursorBlink: false, + cursorStyle: "underline", + }); + run("from js import __py_terminal__ as __terminal__"); + delete globalThis.__py_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 readline here, not sync. + const decoder = new TextDecoder(); + let data = ""; + const generic = { + isatty: true, + write(buffer) { + data = decoder.decode(buffer); + readline.write(data); + return buffer.length; + }, + }; + interpreter.setStdout(generic); + interpreter.setStderr(generic); + interpreter.setStdin({ + isatty: true, + stdin: () => readline.read(data), + }); + + io.stderr = (error) => { + readline.write(`${error.message || error}\n`); + }; + }); + } } }; diff --git a/pyscript.core/src/sync.js b/pyscript.core/src/sync.js index 55519452..3569c762 100644 --- a/pyscript.core/src/sync.js +++ b/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. diff --git a/pyscript.core/test/mpy.spec.js b/pyscript.core/test/mpy.spec.js index 7e89c0f7..b8889102 100644 --- a/pyscript.core/test/mpy.spec.js +++ b/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'); +}); diff --git a/pyscript.core/test/py-terminal-worker.html b/pyscript.core/test/py-terminal-worker.html index 720e51e3..1e4e138e 100644 --- a/pyscript.core/test/py-terminal-worker.html +++ b/pyscript.core/test/py-terminal-worker.html @@ -9,6 +9,7 @@ - + + diff --git a/pyscript.core/test/py-terminals.html b/pyscript.core/test/py-terminals.html new file mode 100644 index 00000000..ff72c551 --- /dev/null +++ b/pyscript.core/test/py-terminals.html @@ -0,0 +1,27 @@ + + + + + + PyTerminal Main + + + + + + + + + diff --git a/pyscript.core/tests/integration/test_py_terminal.py b/pyscript.core/tests/integration/test_py_terminal.py index 8e5e3395..45ee03c1 100644 --- a/pyscript.core/tests/integration/test_py_terminal.py +++ b/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 diff --git a/pyscript.core/types/sync.d.ts b/pyscript.core/types/sync.d.ts index 3dd52650..fb57cb15 100644 --- a/pyscript.core/types/sync.d.ts +++ b/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.