Skip to content

test(core): cover VM disposal after guest fetch - #2014

Open
RitikaxG wants to merge 1 commit into
rivet-dev:mainfrom
RitikaxG:codex/fix-vm-dispose-timer-worker
Open

RitikaxG wants to merge 1 commit into
rivet-dev:mainfrom
RitikaxG:codex/fix-vm-dispose-timer-worker

Conversation

@RitikaxG

@RitikaxG RitikaxG commented Sep 30, 2026 •

Copy link
Copy Markdown
  • Add a fresh-sidecar regression for a guest fetch() whose response body is fully consumed, then require VM disposal before the 5-second shutdown deadline.
  • Run the regression in the required Core PR test lane. It failed on the pre-fix base at 5,047 ms with one VM timer task retained and passes on current main after upstream commit a6c668c made timer-wheel initialization process-owned.

Related: #1989

This covers the completed-response disposal path. The unread-response evaluation delay needs separate investigation.

@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-severity finding

Reviewed commit f26c94d.

Comment thread crates/native-sidecar/src/state.rs Outdated
let mut javascript = JavascriptExecutionEngine::new(process_runtime);
javascript.set_event_notify(Some(Arc::clone(&event_notify)));
let mut python = PythonExecutionEngine::new(runtime.clone());
let mut python = PythonExecutionEngine::new(vm_runtime.clone());

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 · Python can still bind the global timer wheel to one VM

PythonExecutionEngine::new(vm_runtime.clone()) constructs its embedded JavascriptExecutionEngine with this VM-scoped context. Python executions are ultimately started through that engine, whose LocalBridgeState passes the engine context to TimerWheel::get; if Python is the first runtime to schedule a JS/bridge timer, the process-global wheel is therefore still spawned under this VM's admission scope and dies when the VM is disposed, leaving later VMs with the same dead singleton. WasmExecutionEngine::new(vm_runtime) on the next line has the same path. Give both wrapper engines the process runtime for their embedded JavaScript host while retaining the VM runtime for guest execution, and cover first-use through Python/Wasm before disposing the VM.

@RitikaxG
RitikaxG force-pushed the codex/fix-vm-dispose-timer-worker branch from f26c94d to e5aa7f3 Compare October 1, 2026 07:02

@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.

✅ No issues found

Reviewed commit e5aa7f3.

@RitikaxG RitikaxG changed the title fix(execution): keep JavaScript timer worker process-scoped test(core): cover VM disposal after guest fetch Oct 1, 2026

This branch has not been deployed

No deployments
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