fix(external_script): fix crash during concurrent cleanup

pyproject.toml

@@ -51,7 +51,6 @@ include = [
   "/src/trame_client/module/vue2-www/**",
   "/src/trame_client/module/vue3-www/**",
   "/src/trame_client/module/react-www/**",
-  "src/trame_client/module/user_provided_scripts/**",
 ]
 
 [tool.hatch.build.targets.wheel]

src/trame_client/external_script_handler.py

@@ -6,7 +6,6 @@
 from .utils.web_module import file_digest
 
 from .module import (
-    USER_PROVIDED_SCRIPTS_DIR_PATH,
     USER_PROVIDED_SCRIPTS_SERVE_URL_PREFIX,
     USER_PROVIDED_UMD_SCRIPTS_DIR_PATH,
     USER_PROVIDED_ES_SCRIPTS_DIR_PATH,
@@ -15,17 +14,6 @@
 logger = logging.getLogger(__name__)
 
 
-def clean_user_provided_scripts(folder: Path):
-    files_to_keep = {"__init__.py", ".gitignore"}
-
-    for item in folder.iterdir():
-        if item.is_file() and item.name not in files_to_keep:
-            item.unlink()
-
-        if item.is_dir() and item.name not in files_to_keep:
-            clean_user_provided_scripts(item)
-
-
 @dataclass
 class UserDefinedFunction:
     name: str
@@ -124,9 +112,6 @@ def register_user_provided_script(
     if name in EXTERNAL_SCRIPTS_MAP:
         logger.warning("overwriting already registered script %s", name)
 
-    if not EXTERNAL_SCRIPTS_MAP:
-        clean_user_provided_scripts(USER_PROVIDED_SCRIPTS_DIR_PATH)
-
     EXTERNAL_SCRIPTS_MAP[name] = ExternalScript(
         name,
         script_file_path,

src/trame_client/module/__init__.py

@@ -1,10 +1,28 @@
+import atexit
+import os
+import shutil
+import tempfile
 from pathlib import Path
 
-USER_PROVIDED_SCRIPTS_DIR_PATH = Path(__file__).with_name("user_provided_scripts")
+# Registered scripts are copied to a temporary directory that is served to the client and removed at process exit.
+_TMP_DIR_PATH = Path(tempfile.mkdtemp(prefix="trame-client-"))
+_TMP_DIR_PID = os.getpid()
+
+
+@atexit.register
+def _cleanup_tmp_dir():
+    if os.getpid() == _TMP_DIR_PID:
+        shutil.rmtree(_TMP_DIR_PATH, ignore_errors=True)
+
+
+USER_PROVIDED_SCRIPTS_DIR_PATH = _TMP_DIR_PATH
 USER_PROVIDED_UMD_SCRIPTS_DIR_PATH = USER_PROVIDED_SCRIPTS_DIR_PATH / "umd"
 USER_PROVIDED_ES_SCRIPTS_DIR_PATH = USER_PROVIDED_SCRIPTS_DIR_PATH / "es"
 USER_PROVIDED_SCRIPTS_SERVE_URL_PREFIX = Path("__trame_client_external_scripts")
 
+USER_PROVIDED_ES_SCRIPTS_DIR_PATH.mkdir()
+USER_PROVIDED_UMD_SCRIPTS_DIR_PATH.mkdir()
+
 
 def setup_handler_module(server):
     serve_path = USER_PROVIDED_SCRIPTS_DIR_PATH.resolve()

tests/test_external_script_handler.py

@@ -0,0 +1,108 @@
+from pathlib import Path
+
+import multiprocessing
+import pytest
+import traceback
+import uuid
+from playwright.async_api import expect, async_playwright
+from trame.app import get_server
+from unittest.mock import MagicMock
+
+from trame.ui.html import DivLayout
+from trame.widgets import html
+from trame_client.widgets import client
+
+
+def _register_external_script_worker(script_path, name, barrier, errors):
+    from trame_client.widgets import client
+
+    barrier.wait()
+
+    try:
+        client.register_external_script(
+            Path(script_path), function_names=["run"], name=name
+        )
+        errors.put(None)
+    except BaseException:  # noqa: BLE001
+        errors.put(f"{name}:\n{traceback.format_exc()}")
+
+
+def test_register_external_script_is_safe_across_processes(tmp_path):
+    n_workers = 8
+    context = multiprocessing.get_context("spawn")
+    barrier = context.Barrier(n_workers)
+    errors = context.Queue()
+
+    processes = []
+    for index in range(n_workers):
+        script = tmp_path / f"worker_{index}.js"
+        script.write_text(f"export function run_worker_{index}() {{}}\n")
+        processes.append(
+            context.Process(
+                target=_register_external_script_worker,
+                args=(str(script), f"worker_{index}", barrier, errors),
+            )
+        )
+
+    for process in processes:
+        process.start()
+
+    # Drain while running: large tracebacks would fill the pipe and deadlock a join-first.
+    failures = [error for error in (errors.get() for _ in range(n_workers)) if error]
+
+    for process in processes:
+        process.join()
+
+    assert not failures, "Concurrent script registration failed:\n" + "\n".join(
+        failures
+    )
+
+
+@pytest.mark.asyncio
+async def test_registered_external_script_runs_in_the_browser(
+    tmp_path, unused_tcp_port
+):
+    script = tmp_path / "double.js"
+    script.write_text(
+        "export function double(value) {"
+        "  return { status: true, outputs: Number(value) * 2 };"
+        "}\n"
+    )
+
+    server = get_server(f"test_external_script_{uuid.uuid4()}", client_type="vue3")
+    server.state.result = 0
+    mock = MagicMock()
+
+    registered = client.register_external_script(
+        script, function_names=["double"], name="double"
+    )
+
+    def on_completed(outputs):
+        server.state.result = outputs
+        mock(outputs)
+
+    with DivLayout(server):
+        with client.Handler(
+            variable="value_ref",
+            function=registered,
+            completed=(on_completed, "[$event.outputs]"),
+        ):
+            html.Input(v_model="value_ref.value", classes="input")
+        html.Span("{{ result }}", classes="result")
+
+    server.start(port=unused_tcp_port, exec_mode="task")
+    try:
+        await server.ready
+
+        async with async_playwright() as playwright:
+            browser = await playwright.chromium.launch(headless=True)
+            try:
+                page = await browser.new_page()
+                await page.goto(f"http://127.0.0.1:{server.port}/")
+                await page.locator(".input").fill("21")
+                await expect(page.locator(".result")).to_have_text("42")
+                mock.assert_called_once_with(42)
+            finally:
+                await browser.close()
+    finally:
+        await server.stop()