Skip to content

fix(agentos): queue concurrent WebAssembly and Python launches per VM - #2025

Merged
eersnington merged 2 commits into
mainfrom
fix/sidecar-queue-concurrent-launches
Oct 8, 2026
Merged

eersnington merged 2 commits into
mainfrom
fix/sidecar-queue-concurrent-launches

Conversation

@eersnington

@eersnington eersnington commented Oct 8, 2026 •

Copy link
Copy Markdown
Member
  • Each VM keeps its JavaScript, Python, and WebAssembly engines in a RefCell and borrows them with try_borrow_mut, which fails when the engine is already borrowed.
  • WebAssembly and Python launches hold that borrow across the start .await. On a cold VM the start materializes the import cache and prewarms (WebAssembly) or warms Pyodide (Python), so a second launch on the same VM fails with ERR_AGENTOS_VM_EXECUTION_CONFLICT. Warm starts do not yield, so this shows up after a VM wakes, for example when an agent runs two tool calls at once.
  • Each VM now has wasm_launch and python_launch async locks. A launch takes its turn before it borrows the engine and releases it after the start, so a second launch waits instead of failing. Processes still run in parallel once started.
let launch_turn = execution_engines.wasm_launch_turn().await;
let mut wasm_engine = execution_engines.wasm("start WebAssembly execution")?;
// …start the execution (awaits on a cold VM)…
drop(wasm_engine);
drop(launch_turn);
  • Covers the top-level launch in execution/launch.rs and the child launches in execution/child_process.rs (WebAssembly child, nested WebAssembly child, nested Python child). With only the top-level launch, a shell's own child command still hit the conflict.

  • Adds concurrent_launches_on_a_fresh_vm_all_run to crates/client/tests/process_e2e.rs: three WebAssembly and three Python launches started together on a fresh VM all exit 0. On main, all but one launch per runtime fail with ERR_AGENTOS_VM_EXECUTION_CONFLICT.

@railway-app

railway-app Bot commented Oct 8, 2026

Copy link
Copy Markdown

This PR was not deployed automatically as @eersnington does not have access to the Railway project.

In order to get automatic PR deploys, please add @eersnington to your workspace on Railway.

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 1 high · 🟠 1 medium

Reviewed commit 948b9bb.

Comment on lines +5118 to +5120
// A cold start awaits Pyodide warmup while holding the engine; take turns with
// other launches on this VM instead of failing with an execution conflict.
let launch_turn = execution_engines.python_launch_turn().await;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 High · Queue before creating an unregistered process

This await happens after the duplicate-ID check and after spawn_trusted_root_process. If a queued request is cancelled, dropping KernelProcessHandle does not finish or reap that process, so it remains in the VM kernel without an active_processes entry. Two same-runtime requests with the same process_id can also both pass the check; the queue then lets both start, and the later active_processes.insert silently replaces the first. Acquire the runtime turn before creating/reserving the process and hold it through registration, or add a cancellation-safe pending reservation that claims the ID and rolls back the kernel process.

Comment on lines 1149 to +1152
.map_err(|_| execution_engine_conflict_error(&self.inner.vm_id, "Python", operation))
}

pub(crate) async fn python_launch_turn(&self) -> tokio::sync::MutexGuard<'_, ()> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Medium · Engine admission remains bypassable

The queue is independent of python()/wasm(), so existing launch paths can still take the raw RefCell and return the conflict this change is intended to remove. In production, exec_javascript_process_image_owned directly borrows the WASM/Python engines at child_process.rs:5586 and :5620 while another cold start may hold them; the legacy spawn path also queues WASM but still directly borrows Python at :4953. Encapsulate queue acquisition with the engine borrow and route every Python/WASM launch or replacement through that admission path.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not introduced by this pr. but need to look into this deeper

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 1 high · 🟠 1 medium

Reviewed commit f4fe86c.

Comment thread crates/native-sidecar/src/execution/launch.rs
Comment thread crates/native-sidecar/src/state.rs
@eersnington
eersnington force-pushed the fix/sidecar-queue-concurrent-launches branch from f4fe86c to 249dcd9 Compare October 8, 2026 19:10

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 1 medium-severity finding

Reviewed commit 249dcd9.

Comment thread crates/native-sidecar/src/execution/launch.rs
@eersnington
eersnington merged commit cbdd2ce into main Oct 8, 2026
3 of 5 checks passed
@eersnington
eersnington deleted the fix/sidecar-queue-concurrent-launches branch October 8, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant