feat: process isolation for custom nodes via pyisolate#13324
feat: process isolation for custom nodes via pyisolate#13324pollockjj merged 8 commits intoComfy-Org:pyisolate-supportfrom
Conversation
Adds the isolation system foundation: ComfyUIAdapter, extension loader, manifest discovery, child/host process hooks, RPC bridge, runtime helpers, SHM forensics, and the --use-process-isolation CLI flag. pyisolate added to requirements.txt. .pyisolate_venvs/ added to .gitignore.
Adds proxy infrastructure for cross-process service access: BaseRegistry/ BaseProxy pattern, FolderPaths, ModelManagement, PromptServer, Progress, Utils, HelperProxies, and WebDirectory proxies. These provide transparent RPC access to host-side ComfyUI services from isolated child processes.
…lSampling Adds complex model proxies that handle cross-process model operations: ModelPatcherProxy/Registry with VRAM headroom pre-allocation, CLIPProxy with tokenizer/cond-stage sub-proxies, VAEProxy with encode/decode, ModelSamplingProxy with sigma conversion. Hook serialization for LoRA and weight patches.
Wires isolation into ComfyUI's execution pipeline: child process startup in main.py, isolated node dispatch in execution.py with boundary cleanup, graph notification, quiescence waits, and RPC event loop coordination. Integrates with master's try/finally and RAM pressure structures.
Adds torch-free serializers for sealed workers: ndarray (base64), PLY (point clouds), NPZ (depth frames), TRIMESH (meshes), SKELETON (geometry). comfy_api_sealed_worker package for V1-style sealed node type definitions. SaveNPZ/SavePLY nodes. comfy_api _ui.py child-process detection.
Adds host_policy.py for loading sandbox config from pyproject.toml: sandbox_mode, network access, writable/readonly paths, node whitelist. Sealed-worker RO import paths. Policy validation against forbidden path set. pyproject.toml [tool.comfy.host] section with default policy.
Every isolation code path in ComfyUI core is fenced behind args.use_process_isolation or PYISOLATE_CHILD env checks. This commit contains all scatter hooks across nodes.py, server.py, cuda_malloc.py, comfy/hooks.py, comfy/samplers.py, comfy/model_base.py, comfy/model_management.py, and comfy/k_diffusion/sampling.py. Zero behavioral change when --use-process-isolation is not passed.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in --use-process-isolation mode that runs custom nodes inside isolated child processes via pyisolate, while keeping heavyweight model objects (ModelPatcher/CLIP/VAE/etc.) on the host behind RPC proxies. This includes sandbox/policy support, host/child hooks, proxy infrastructure, and a large isolation-focused test suite.
Changes:
- Introduce
comfy.isolationinfrastructure: adapter, manifest discovery/cache, host/child hooks, RPC bridge, singleton/model proxies, and SHM forensics. - Integrate isolation into startup, custom node loading, execution, and extension web asset serving.
- Add sealed-worker (torch-free) data types + workflows, and extensive unit/integration tests for isolation contracts.
Reviewed changes
Copilot reviewed 84 out of 86 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_adapter.py | Adapter/path/serializer coverage + tmp isolation fence test |
| tests/isolation/workflows/quick_8_conda_sealed_worker.json | Workflow fixture for conda sealed worker |
| tests/isolation/workflows/quick_6_uv_sealed_worker.json | Workflow fixture for uv sealed worker |
| tests/isolation/workflows/isolation_9_conda_sealed_worker.json | Workflow fixture for conda sealed worker |
| tests/isolation/workflows/isolation_7_uv_sealed_worker.json | Workflow fixture for uv sealed worker |
| tests/isolation/workflows/internal_probe_ui3d.json | Internal probe workflow fixture |
| tests/isolation/workflows/internal_probe_preview_image_audio.json | Internal probe workflow fixture |
| tests/isolation/uv_sealed_worker/pyproject.toml | Test sealed-worker manifest (uv) |
| tests/isolation/uv_sealed_worker/init.py | Test sealed-worker node implementations (uv) |
| tests/isolation/test_web_directory_proxy.py | Unit tests for web directory proxy allow-list/traversal/content |
| tests/isolation/test_web_directory_handler.py | Unit tests for host cache + handler behavior |
| tests/isolation/test_singleton_proxy_boundary_matrix.py | Contract tests for “sealed singleton” import boundaries |
| tests/isolation/test_shared_model_proxy_contract.py | Contract test for shared ModelPatcher proxy behavior |
| tests/isolation/test_savedimages_serialization.py | Serialization roundtrip test for UI SavedImages |
| tests/isolation/test_runtime_helpers_stub_contract.py | Contract tests for runtime helper stubs and staging |
| tests/isolation/test_path_helpers.py | Tests for sys.path snapshot/unification helpers |
| tests/isolation/test_model_management_proxy.py | Unit tests for ModelManagementProxy device APIs |
| tests/isolation/test_manifest_loader_discovery.py | Tests for manifest discovery (including nested standalone) |
| tests/isolation/test_internal_probe_node_loading.py | Tests for staging/loading internal probe nodes and child bootstrap |
| tests/isolation/test_internal_probe_node_assets.py | Tests ensuring probe assets/workflows are minimal and toolkit-free |
| tests/isolation/test_init.py | Tests for comfy.isolation initialization and sealed-child safety |
| tests/isolation/test_host_policy.py | Tests for host policy parsing/validation and overrides |
| tests/isolation/test_folder_paths_proxy.py | Unit tests for FolderPathsProxy + sealed-child RPC behavior |
| tests/isolation/test_extension_loader_sealed_worker.py | Tests for sealed-worker loader selection + policy RO paths |
| tests/isolation/test_exact_proxy_relay_matrix.py | Exact-relay transcript verification for multiple proxies |
| tests/isolation/test_exact_proxy_bootstrap_contract.py | Contract test ensuring proxy bootstrap completeness |
| tests/isolation/test_client_snapshot.py | Tests for pyisolate client snapshot sys.path behavior |
| tests/isolation/stage_internal_probe_node.py | Utility to stage internal probe node under an explicit root |
| tests/isolation/internal_probe_node/probe_nodes.py | Internal probe node implementations (image/audio/ui3d previews) |
| tests/isolation/internal_probe_node/init.py | Internal probe package exports |
| tests/isolation/internal_probe_host_policy.toml | Host policy fixture for internal probe workflows |
| tests/isolation/conda_sealed_worker/pyproject.toml | Test sealed-worker manifest (conda) |
| tests/isolation/conda_sealed_worker/init.py | Test sealed-worker node implementations (conda/cfgrib/xarray) |
| server.py | Serve isolated extension web assets + include proxied JS in /extensions |
| requirements.txt | Add pyisolate==0.10.0 dependency |
| pyproject.toml | Add [tool.comfy.host] policy defaults + whitelist |
| nodes.py | Integrate isolation loading, claimed-path skipping, and whitelist enforcement |
| main.py | Isolation bootstrap/adapter registration + child/primary fencing changes |
| execution.py | Isolated node dispatch path + boundary cleanup/quiescence hooks |
| cuda_malloc.py | Avoid appending cudaMallocAsync backend when isolation enabled |
| comfy/samplers.py | Device placement tweaks when isolation is active |
| comfy/model_management.py | Isolation-aware memory cleanup + safer finalizer handling |
| comfy/model_base.py | Add pickling reduction for ModelSampling under isolation |
| comfy/k_diffusion/sampling.py | Device alignment for sampling when isolation is active |
| comfy/isolation/vae_proxy.py | VAE/FirstStageModel proxy + registries |
| comfy/isolation/shm_forensics.py | /dev/shm torch_* tracking for diagnostics |
| comfy/isolation/rpc_bridge.py | Sync runner for coroutines inside isolated processes |
| comfy/isolation/proxies/web_directory_proxy.py | Web directory RPC proxy + host cache implementation |
| comfy/isolation/proxies/utils_proxy.py | Utils proxy for PROGRESS_BAR_HOOK relay |
| comfy/isolation/proxies/prompt_server_impl.py | PromptServer service/stub for host/child RPC routes + UI messages |
| comfy/isolation/proxies/progress_proxy.py | Progress proxy for updating host progress state |
| comfy/isolation/proxies/model_management_proxy.py | Exact-relay proxy for comfy.model_management |
| comfy/isolation/proxies/helper_proxies.py | Serialization helpers for AnyType/FlexibleOptional/ByPassTypeTuple |
| comfy/isolation/proxies/folder_paths_proxy.py | FolderPaths proxy with explicit RPC property methods |
| comfy/isolation/proxies/base.py | Base registry/proxy + sync RPC execution helpers |
| comfy/isolation/proxies/init.py | Proxy package exports |
| comfy/isolation/model_sampling_proxy.py | Proxy/registry for ModelSampling methods and properties |
| comfy/isolation/model_patcher_proxy_utils.py | ModelPatcher isolation wrapping + hook serializer registration |
| comfy/isolation/manifest_loader.py | Manifest discovery and cache keying for isolated extensions |
| comfy/isolation/host_policy.py | Host policy parsing/normalization/validation |
| comfy/isolation/host_hooks.py | Host-side proxy/service initialization for isolation |
| comfy/isolation/custom_node_serializers.py | Legacy no-op serializer shim for compatibility |
| comfy/isolation/clip_proxy.py | CLIP proxy + registries for patcher/cond_stage/tokenizer |
| comfy/isolation/child_hooks.py | Child-side init: extra paths, loop bridge, and proxy caller wiring |
| comfy/hooks.py | Hook ref identity stability under isolation + weight dict guard |
| comfy/cli_args.py | Add --use-process-isolation CLI flag |
| comfy_extras/nodes_save_ply.py | Add SavePLY output node using sealed-worker PLY type |
| comfy_extras/nodes_save_npz.py | Add SaveNPZ output node using sealed-worker NPZ type |
| comfy_api/latest/_ui.py | Route preview outputs to output folder in isolated child mode |
| comfy_api_sealed_worker/trimesh_types.py | Torch-free trimesh payload type |
| comfy_api_sealed_worker/ply_types.py | Torch-free PLY payload type |
| comfy_api_sealed_worker/npz_types.py | Torch-free NPZ payload type |
| comfy_api_sealed_worker/init.py | Export sealed-worker types package |
| .gitignore | Ignore .pyisolate_venvs/ |
Comments suppressed due to low confidence (1)
main.py:85
comfy_aimdo.controlis imported and initialized twice (once around the early DynamicVRAM check, and again later). Double initialization can cause redundant work or subtle side effects. Consolidate to a single import/init block (and keep the early-failure handling if needed).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cache = get_web_directory_cache() | ||
| result = cache.get_file(ext_name, file_path) | ||
| if result is None: | ||
| return web.Response(status=404, text="Not found") | ||
|
|
||
| content_type = { | ||
| ".js": "application/javascript", | ||
| ".css": "text/css", | ||
| ".html": "text/html", | ||
| ".json": "application/json", | ||
| }.get(suffix, "application/octet-stream") | ||
|
|
||
| return web.Response(body=result, content_type=content_type) | ||
|
|
There was a problem hiding this comment.
cache.get_file() returns a dict with keys like content (bytes) and content_type, but the handler passes the whole dict as the aiohttp response body. This will raise at runtime (body must be bytes/str/etc). Use the cached bytes payload (and ideally the cached content_type) when building the web.Response.
| suffix = os.path.splitext(file_path)[1].lower() | ||
| if suffix not in ALLOWED_EXTENSIONS: | ||
| return web.Response(status=403, text="Forbidden file type") | ||
|
|
||
| cache = get_web_directory_cache() | ||
| result = cache.get_file(ext_name, file_path) | ||
| if result is None: | ||
| return web.Response(status=404, text="Not found") | ||
|
|
||
| content_type = { | ||
| ".js": "application/javascript", | ||
| ".css": "text/css", | ||
| ".html": "text/html", | ||
| ".json": "application/json", | ||
| }.get(suffix, "application/octet-stream") |
There was a problem hiding this comment.
The handler blocks any suffix not in ALLOWED_EXTENSIONS (currently .js/.html/.css), but the content_type map includes .json and an octet-stream fallback. This mapping is unreachable as written; either extend ALLOWED_EXTENSIONS to match the intended types or remove the dead entries and rely on result["content_type"].
| # Include JS files from proxied web directories (isolated nodes) | ||
| if args.use_process_isolation: | ||
| from comfy.isolation.proxies.web_directory_proxy import get_web_directory_cache | ||
| cache = get_web_directory_cache() | ||
| for ext_name in cache.extension_names: | ||
| for entry in cache.list_files(ext_name): | ||
| if entry["relative_path"].endswith(".js"): | ||
| extensions.append( | ||
| "/extensions/" + urllib.parse.quote(ext_name) + "/" + entry["relative_path"] | ||
| ) |
There was a problem hiding this comment.
Proxied extension URLs are built by quoting only ext_name, but entry["relative_path"] is appended without URL encoding. If an extension contains spaces or other reserved characters in filenames, the generated URL can break or be interpreted inconsistently. Consider URL-encoding each path segment (or at least relative_path) when appending to /extensions/{ext}/....
| target = (root / relative_path).resolve() | ||
|
|
||
| # Ensure resolved path is under the web directory | ||
| if not str(target).startswith(str(root.resolve())): |
There was a problem hiding this comment.
The path containment check uses str(target).startswith(str(root.resolve())), which can be bypassed via symlinks or prefix collisions (e.g. /web vs /web_evil). Use a path-based check (e.g. target.is_relative_to(root_resolved) / target.relative_to(root_resolved) inside a try) against a resolved root to reliably prevent escaping the web directory.
| target = (root / relative_path).resolve() | |
| # Ensure resolved path is under the web directory | |
| if not str(target).startswith(str(root.resolve())): | |
| root_resolved = root.resolve() | |
| target = (root / relative_path).resolve() | |
| # Ensure resolved path is under the resolved web directory | |
| try: | |
| target.relative_to(root_resolved) | |
| except ValueError: |
| def get_filename_list(self, folder_name: str) -> list[str]: | ||
| caller_stack = "".join(traceback.format_stack()[-4:-1]) | ||
| _fp_logger.warning( | ||
| "][ DIAG:FolderPathsProxy.get_filename_list called | folder=%s | is_child=%s | rpc_configured=%s\n%s", | ||
| folder_name, _is_child_process(), self._rpc is not None, caller_stack, | ||
| ) | ||
| if _is_child_process(): | ||
| result = list(call_singleton_rpc(self._get_caller(), "rpc_get_filename_list", folder_name)) | ||
| _fp_logger.warning( | ||
| "][ DIAG:FolderPathsProxy.get_filename_list RPC result | folder=%s | count=%d | first=%s", | ||
| folder_name, len(result), result[:3] if result else "EMPTY", | ||
| ) | ||
| return result | ||
| result = list(_folder_paths().get_filename_list(folder_name)) | ||
| _fp_logger.warning( | ||
| "][ DIAG:FolderPathsProxy.get_filename_list LOCAL result | folder=%s | count=%d | first=%s", | ||
| folder_name, len(result), result[:3] if result else "EMPTY", | ||
| ) | ||
| return result |
There was a problem hiding this comment.
get_filename_list() logs multiple WARNING-level diagnostic messages (including stack traces) on every call. This is likely to be extremely noisy in production and can leak filesystem/layout details into logs. Gate these diagnostics behind an env flag and/or drop them to DEBUG level.
| def initialize_child_process() -> None: | ||
| logger.warning("][ DIAG:child_hooks initialize_child_process START") | ||
| if os.environ.get("PYISOLATE_IMPORT_TORCH", "1") != "0": | ||
| _load_extra_model_paths() | ||
| _setup_child_loop_bridge() | ||
|
|
||
| # Manual RPC injection | ||
| try: | ||
| from pyisolate._internal.rpc_protocol import get_child_rpc_instance | ||
|
|
||
| rpc = get_child_rpc_instance() | ||
| logger.warning("][ DIAG:child_hooks RPC instance: %s", rpc is not None) | ||
| if rpc: | ||
| _setup_proxy_callers(rpc) | ||
| logger.warning("][ DIAG:child_hooks proxy callers configured with RPC") | ||
| else: | ||
| logger.warning("][ DIAG:child_hooks NO RPC — proxy callers cleared") | ||
| _setup_proxy_callers() | ||
| except Exception as e: | ||
| logger.error(f"][ DIAG:child_hooks Manual RPC Injection failed: {e}") | ||
| _setup_proxy_callers() | ||
|
|
||
| _setup_logging() |
There was a problem hiding this comment.
Child init logs multiple WARNING-level DIAG: messages unconditionally. This will spam logs for every isolated process and can expose internal state. Consider making these debug-only or gating behind an env var (similar to other diagnostics).
| [tool.comfy.host] | ||
| sandbox_mode = "disabled" | ||
| allow_network = false | ||
| writable_paths = ["/dev/shm", "/tmp"] |
There was a problem hiding this comment.
writable_paths includes /tmp, but comfy.isolation.host_policy explicitly filters /tmp out via FORBIDDEN_WRITABLE_PATHS. This configuration entry is therefore ignored at runtime and is misleading (and suggests a weaker default policy than is actually enforced). Remove /tmp here or change the forbidden list/policy logic if /tmp is intentionally allowed.
| if '--use-process-isolation' in sys.argv: | ||
| from comfy.isolation import initialize_proxies | ||
| initialize_proxies() | ||
|
|
There was a problem hiding this comment.
Isolation setup is triggered by checking '--use-process-isolation' in sys.argv even though args.use_process_isolation is already available after enable_args_parsing(). Using sys.argv here can diverge from parsed args (aliases, config, programmatic invocation) and makes testing harder. Prefer if args.use_process_isolation:.
Process Isolation for Custom Nodes
Adds
--use-process-isolationflag that runs custom nodes in isolated child processes via pyisolate. Models, CLIP, VAE stay on the host — child processes get lightweight RPC proxies. Zero behavioral change when the flag is not passed.Commits
args.use_process_isolationorPYISOLATE_CHILDchecksVerification
--use-process-isolationis not passed