fix: use shared memory for virtual files when running with app isolation #9181
fix: use shared memory for virtual files when running with app isolation #9181
Conversation
This change updates isolated apps to use shared memory for virtual file storage. Isolated apps run in different processes from the main server. This means they need to place virtual file data in shared memory, but historically run-mode has used run in-process and used in-memory storage. The change looks large because we're lifting the choice of storage out of the kernel context, letting the caller decide what storage backend to use. This required touching many files.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| # Use shared memory in edit mode, | ||
| # in-memory storage in run mode (same process) | ||
| storage = ( | ||
| # Storage is chosen explicitly by the caller. None means virtual files | ||
| # are not supported; we still construct an (inert) InMemoryStorage so | ||
| # the registry has a backend, but ctx.virtual_files_supported is False. | ||
| storage: VirtualFileStorage = ( | ||
| SharedMemoryStorage() | ||
| if mode == SessionMode.EDIT | ||
| if virtual_file_storage == "shared_memory" | ||
| else InMemoryStorage() | ||
| ) |
There was a problem hiding this comment.
This is the main change in this PR.
There was a problem hiding this comment.
Pull request overview
This PR updates marimo’s virtual file subsystem to work correctly when apps are run in isolated subprocesses by making the virtual-file storage backend an explicit choice (shared memory vs in-process memory vs disabled).
Changes:
- Replace the boolean
virtual_files_supportedflag withvirtual_file_storage: "in_memory" | "shared_memory" | Noneand thread it through session/kernel/app-host/runtime creation paths. - Configure AppHost (isolated run-mode) kernels to use shared-memory virtual-file storage so the parent server can serve
/@file/...requests. - Add a process-isolation smoke test for virtual files and update docs/tests to use the new parameter.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/conftest.py | Updates kernel/test config to pass virtual_file_storage based on session mode. |
| tests/_session/app_host/test_main.py | Switches app-host tests to pass virtual_file_storage="shared_memory". |
| tests/_session/app_host/test_app_host.py | Switches app-host tests to pass virtual_file_storage="shared_memory". |
| tests/_server/test_sessions.py | Updates session/kernel-manager tests to new virtual_file_storage arg. |
| tests/_runtime/test_runtime.py | Updates runtime tests to use virtual_file_storage. |
| marimo/_smoke_tests/process_isolation/virtual_files.py | Adds a smoke-test notebook for virtual files under process isolation. |
| marimo/_smoke_tests/process_isolation/README.md | Documents how to run the process-isolation virtual-file smoke test. |
| marimo/_session/session.py | Threads virtual_file_storage into the “original kernel” session creation path. |
| marimo/_session/managers/kernel.py | KernelManager now stores and passes virtual_file_storage into runtime.launch_kernel. |
| marimo/_session/managers/ipc.py | Removes virtual_files_supported from IPC manager args (IPC kernels always disable virtual files). |
| marimo/_session/managers/app_host.py | Forces AppHost kernels to use virtual_file_storage="shared_memory". |
| marimo/_session/app_host/main.py | App-host kernel launch now forwards virtual_file_storage. |
| marimo/_session/app_host/host.py | AppHost create_kernel now accepts virtual_file_storage. |
| marimo/_session/app_host/commands.py | Updates CreateKernelCmd to include virtual_file_storage. |
| marimo/_server/session_manager.py | Chooses default virtual_file_storage based on session mode (edit vs run). |
| marimo/_server/export/init.py | Export path disables virtual files via virtual_file_storage=None. |
| marimo/_runtime/virtual_file/storage.py | Introduces VirtualFileStorageType and keeps storage backends. |
| marimo/_runtime/virtual_file/init.py | Re-exports VirtualFileStorageType. |
| marimo/_runtime/runtime.py | Updates launch_kernel signature to accept virtual_file_storage. |
| marimo/_runtime/context/kernel_context.py | Selects virtual-file backend based on virtual_file_storage and sets virtual_files_supported. |
| marimo/_runtime/app/kernel_runner.py | Uses shared-memory storage for embedded/edit-like runner context. |
| marimo/_pyodide/pyodide_session.py | Disables virtual files via virtual_file_storage=None. |
| marimo/_ipc/types.py | Removes virtual_files_supported from KernelArgs. |
| marimo/_ipc/launch_kernel.py | IPC kernel launch disables virtual files via virtual_file_storage=None. |
| config_manager=config_manager, | ||
| virtual_files_supported=True, | ||
| virtual_file_storage="shared_memory", | ||
| redirect_console_to_browser=False, | ||
| ttl_seconds=None, | ||
| auto_instantiate=auto_instantiate, |
There was a problem hiding this comment.
create_session() hardcodes virtual_file_storage="shared_memory" for both EDIT and RUN sessions. That means the RUN-mode branches in this test are not exercising the default in-process storage behavior. Consider selecting "in_memory" when mode == SessionMode.RUN (and "shared_memory" for EDIT) so the helper reflects real configuration.
| # Storage is chosen explicitly by the caller. None means virtual files | ||
| # are not supported; we still construct an (inert) InMemoryStorage so | ||
| # the registry has a backend, but ctx.virtual_files_supported is False. | ||
| storage: VirtualFileStorage = ( | ||
| SharedMemoryStorage() | ||
| if mode == SessionMode.EDIT | ||
| if virtual_file_storage == "shared_memory" | ||
| else InMemoryStorage() | ||
| ) |
There was a problem hiding this comment.
create_kernel_context treats any non-"shared_memory" value (including typos) as in-memory storage, while still marking virtual_files_supported as true for any non-None value. This can silently mask misconfiguration and lead to hard-to-debug cross-process 404s. Consider validating virtual_file_storage explicitly (e.g., handle "in_memory" and "shared_memory" and raise on unknown values) before selecting the backend.
| app_metadata=app_metadata, | ||
| config_manager=get_default_config_manager(current_path=None), | ||
| virtual_files_supported=True, | ||
| virtual_file_storage="shared_memory", |
There was a problem hiding this comment.
This RUN-mode session test passes virtual_file_storage="shared_memory", but RUN mode kernels are in-process and should normally use "in_memory". Keeping the test aligned with the real configuration makes it more likely to catch accidental behavior changes and avoids unnecessary shared-memory usage.
| virtual_file_storage="shared_memory", | |
| virtual_file_storage="in_memory", |
| virtual_file_storage="shared_memory", | ||
| redirect_console_to_browser=False, |
There was a problem hiding this comment.
This RUN-mode session test passes virtual_file_storage="shared_memory", but RUN mode kernels are in-process and should normally use "in_memory". Using shared memory here can reduce the test’s fidelity to production behavior.
| app_metadata=app_metadata, | ||
| config_manager=get_default_config_manager(current_path=None), | ||
| virtual_files_supported=True, | ||
| virtual_file_storage="shared_memory", |
There was a problem hiding this comment.
This RUN-mode session test passes virtual_file_storage="shared_memory", but RUN mode kernels are in-process and should normally use "in_memory". Consider updating to in-memory storage to better reflect real run-mode configuration.
| virtual_file_storage="shared_memory", | |
| virtual_file_storage="in_memory", |
| marimo/_smoke_tests/process_isolation/app1.py | ||
| ``` | ||
|
|
||
| Multi-file `marimo run` auto-enables process isolation. If all three |
There was a problem hiding this comment.
This text says “all three sections below” but the notebook only has two sections (image + table). Update the wording or add the missing third section so the smoke test instructions are consistent.
| Multi-file `marimo run` auto-enables process isolation. If all three | |
| Multi-file `marimo run` auto-enables process isolation. If both |
| app_metadata=app_metadata, | ||
| config_manager=get_default_config_manager(current_path=None), | ||
| virtual_files_supported=True, | ||
| virtual_file_storage="shared_memory", |
There was a problem hiding this comment.
This test configures a RUN-mode (threaded) kernel with virtual_file_storage="shared_memory". Production RUN mode defaults to in-process storage, so using shared memory here can hide regressions and adds an unnecessary platform dependency. Consider switching this RUN-mode test to virtual_file_storage="in_memory" to match the intended behavior.
| virtual_file_storage="shared_memory", | |
| virtual_file_storage="in_memory", |
| This notebook creates virtual files (HTML, image, Arrow) to verify they | ||
| are served correctly when the app runs in an isolated subprocess | ||
| (`isolate_apps=true`). | ||
|
|
||
| **Bug:** When `isolate_apps` is enabled, the kernel runs in a child | ||
| process that stores virtual files in process-local `InMemoryStorage`. | ||
| The server process cannot access them, causing `FileNotFoundError` | ||
| when the browser requests `/@file/...`. |
There was a problem hiding this comment.
The intro markdown says this notebook creates virtual files for “HTML, image, Arrow” and later refers to “empty iframes”, but the cells below only create an image and a table (Arrow). Either add an HTML-based output that actually exercises the HTML virtual-file path, or update the text so the smoke test description matches what it covers.
| ``` | ||
|
|
||
| Multi-file `marimo run` auto-enables process isolation. You should see a | ||
| blue square image, a green HTML heading, and a three-row table. If any of |
There was a problem hiding this comment.
The expected-results text mentions “a green HTML heading”, but the referenced notebooks (app1 + virtual_files) appear to render markdown/callouts, not an HTML virtual file. Either adjust the expectation (e.g., “green PASS callout heading”) or add an explicit HTML virtual-file output to the smoke test so this instruction is accurate.
| blue square image, a green HTML heading, and a three-row table. If any of | |
| blue square image, a green PASS callout heading, and a three-row table. If any of |
| app_file_manager=app_file_manager, | ||
| config_manager=get_default_config_manager(current_path=None), | ||
| virtual_files_supported=True, | ||
| virtual_file_storage="shared_memory", |
There was a problem hiding this comment.
This session is created in RUN mode but is configured with virtual_file_storage="shared_memory". Since RUN mode kernels are in-process by default, using "in_memory" here would better match production behavior and avoid relying on shared-memory support when it isn’t needed.
| virtual_file_storage="shared_memory", | |
| virtual_file_storage="in_memory", |
This change updates isolated apps to use shared memory for virtual
file storage.
Isolated apps run in different processes from the main server. This
means they need to place virtual file data in shared memory, but
historically run-mode has run in-process and used in-memory
storage.
The change looks large because we're lifting the choice of storage
out of the kernel context, letting the caller decide what storage
backend to use. This required touching many files. In practice the change is small.