Skip to content

Commit ca4e88d

Browse files
committed
chore: US-028 V8 event loop infrastructure for Pi interactive TUI
V8 sidecar improvements for interactive TUI support: - sync_call call_id matching to handle interleaved async responses - ResponseReceiver::defer() for non-matching BridgeResponse routing - MODULE_RESOLVE_STATE persists through event loop for dynamic import - V8 crate upgraded to v134 (from v130) - Improved error reporting with stderr in IPC close messages Pi interactive TUI still blocked by V8 microtask checkpoint hang (perform_microtask_checkpoint blocks on TUI render cycles).
1 parent 3a428a7 commit ca4e88d

8 files changed

Lines changed: 145 additions & 33 deletions

File tree

native/v8-runtime/Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

native/v8-runtime/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ name = "secure-exec-v8"
1010
path = "src/main.rs"
1111

1212
[dependencies]
13-
v8 = "130"
13+
v8 = "134"
1414
crossbeam-channel = "0.5"
1515
signal-hook = "0.3"
1616
libc = "0.2"

native/v8-runtime/src/bridge.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -528,8 +528,7 @@ pub fn resolve_pending_promise(
528528
resolver.resolve(scope, undef.into());
529529
}
530530

531-
// Flush microtasks after resolution
532-
scope.perform_microtask_checkpoint();
531+
// Microtask checkpoint is the caller's responsibility (explicit policy).
533532

534533
Ok(())
535534
}

native/v8-runtime/src/execution.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,7 @@ pub fn execute_script(
378378
None => (1, None),
379379
};
380380
}
381+
381382
}
382383

383384
if bridge_ctx.is_some() {
@@ -562,12 +563,12 @@ fn build_os_config<'s>(
562563

563564
/// Thread-local state for module resolution during execute_module.
564565
/// Avoids passing user data through V8's ResolveModuleCallback (which is a plain fn pointer).
565-
struct ModuleResolveState {
566-
bridge_ctx: *const BridgeCallContext,
566+
pub(crate) struct ModuleResolveState {
567+
pub(crate) bridge_ctx: *const BridgeCallContext,
567568
/// identity_hash → resource_name for referrer lookup
568-
module_names: HashMap<NonZeroI32, String>,
569+
pub(crate) module_names: HashMap<NonZeroI32, String>,
569570
/// resolved_path → Global<Module> cache
570-
module_cache: HashMap<String, v8::Global<v8::Module>>,
571+
pub(crate) module_cache: HashMap<String, v8::Global<v8::Module>>,
571572
}
572573

573574
// SAFETY: ModuleResolveState is only accessed from the session thread
@@ -576,7 +577,7 @@ struct ModuleResolveState {
576577
unsafe impl Send for ModuleResolveState {}
577578

578579
thread_local! {
579-
static MODULE_RESOLVE_STATE: RefCell<Option<ModuleResolveState>> = const { RefCell::new(None) };
580+
pub(crate) static MODULE_RESOLVE_STATE: RefCell<Option<ModuleResolveState>> = const { RefCell::new(None) };
580581
}
581582

582583
fn clear_module_state() {

native/v8-runtime/src/host_call.rs

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ impl FrameSender for WriterFrameSender {
6262
/// Production code uses a channel-based implementation; tests use a buffer-based one.
6363
pub trait ResponseReceiver: Send {
6464
fn recv_response(&self) -> Result<BinaryFrame, String>;
65+
/// Defer a frame for later processing (used when sync_call receives
66+
/// a BridgeResponse for a different call_id).
67+
fn defer(&self, frame: BinaryFrame);
6568
}
6669

6770
/// ResponseReceiver that reads frames from a byte buffer via ipc_binary::read_frame.
@@ -81,6 +84,10 @@ impl ReaderResponseReceiver {
8184
}
8285

8386
impl ResponseReceiver for ReaderResponseReceiver {
87+
fn defer(&self, _frame: BinaryFrame) {
88+
// Test-only receiver — deferred frames are dropped
89+
}
90+
8491
fn recv_response(&self) -> Result<BinaryFrame, String> {
8592
let mut reader = self.reader.lock().unwrap();
8693
ipc_binary::read_frame(&mut *reader)
@@ -132,6 +139,10 @@ impl FrameSender for StubFrameSender {
132139
struct StubResponseReceiver;
133140

134141
impl ResponseReceiver for StubResponseReceiver {
142+
fn defer(&self, _frame: BinaryFrame) {
143+
panic!("stub bridge function called during snapshot creation")
144+
}
145+
135146
fn recv_response(&self) -> Result<BinaryFrame, String> {
136147
panic!("stub bridge function called during snapshot creation — bridge IIFE must not call bridge functions at setup time")
137148
}
@@ -227,35 +238,42 @@ impl BridgeCallContext {
227238
return Err(format!("failed to write BridgeCall: {}", e));
228239
}
229240

230-
// Receive BridgeResponse directly (no re-serialization)
241+
// Receive BridgeResponse matching our call_id.
242+
// Non-matching BridgeResponses (from async bridge calls like timers)
243+
// are deferred for later processing by the event loop.
231244
let response = {
232245
let rx = self.response_rx.lock().unwrap();
233-
match rx.recv_response() {
234-
Ok(frame) => frame,
235-
Err(e) => {
236-
self.pending_calls.lock().unwrap().remove(&call_id);
237-
return Err(e);
246+
loop {
247+
match rx.recv_response() {
248+
Ok(frame) => {
249+
match &frame {
250+
BinaryFrame::BridgeResponse { call_id: resp_id, .. } if *resp_id == call_id => {
251+
break frame;
252+
}
253+
_ => {
254+
// Non-matching response — defer for event loop
255+
rx.defer(frame);
256+
}
257+
}
258+
}
259+
Err(e) => {
260+
self.pending_calls.lock().unwrap().remove(&call_id);
261+
return Err(e);
262+
}
238263
}
239264
}
240265
};
241266

242267
// Remove from pending
243268
self.pending_calls.lock().unwrap().remove(&call_id);
244269

245-
// Validate and extract BridgeResponse
270+
// Extract BridgeResponse
246271
match response {
247272
BinaryFrame::BridgeResponse {
248-
call_id: resp_id,
249273
status,
250274
payload,
251275
..
252276
} => {
253-
if resp_id != call_id {
254-
return Err(format!(
255-
"call_id mismatch: expected {}, got {}",
256-
call_id, resp_id
257-
));
258-
}
259277
if status == 1 {
260278
// Error: payload is UTF-8 error message
261279
Err(String::from_utf8_lossy(&payload).to_string())
@@ -266,7 +284,7 @@ impl BridgeCallContext {
266284
Ok(Some(payload))
267285
}
268286
}
269-
_ => Err("expected BridgeResponse, got different message type".into()),
287+
_ => unreachable!("loop only breaks on BridgeResponse"),
270288
}
271289
}
272290

native/v8-runtime/src/session.rs

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -511,18 +511,33 @@ fn session_thread(
511511
)
512512
};
513513

514+
// Re-initialize module resolve state for the event loop.
515+
// execute_script/execute_module clear MODULE_RESOLVE_STATE
516+
// on return, but dynamic import() calls during the event loop
517+
// (e.g. from timer callbacks) need it to resolve modules.
518+
execution::MODULE_RESOLVE_STATE.with(|cell| {
519+
if cell.borrow().is_none() {
520+
*cell.borrow_mut() = Some(execution::ModuleResolveState {
521+
bridge_ctx: &bridge_ctx as *const _,
522+
module_names: std::collections::HashMap::new(),
523+
module_cache: std::collections::HashMap::new(),
524+
});
525+
}
526+
});
527+
514528
// Run event loop if there are pending async promises
515529
let mut terminated = if pending.len() > 0 {
516530
let scope = &mut v8::HandleScope::new(iso);
517531
let ctx = v8::Local::new(scope, &exec_context);
518532
let scope = &mut v8::ContextScope::new(scope, ctx);
519-
!run_event_loop(
533+
let result = run_event_loop(
520534
scope,
521535
&rx,
522536
&pending,
523537
maybe_abort_rx.as_ref(),
524538
Some(&deferred_queue),
525-
)
539+
);
540+
!result
526541
} else {
527542
false
528543
};
@@ -560,6 +575,11 @@ fn session_thread(
560575
}
561576
}
562577

578+
// Clear module resolve state after event loop completes
579+
execution::MODULE_RESOLVE_STATE.with(|cell| {
580+
*cell.borrow_mut() = None;
581+
});
582+
563583
// Check if timeout fired
564584
let timed_out = timeout_guard.as_ref().is_some_and(|g| g.timed_out());
565585

@@ -737,7 +757,6 @@ pub(crate) fn run_event_loop(
737757
Err(_) => return false,
738758
},
739759
recv(abort) -> _ => {
740-
// Timeout fired — abort channel closed
741760
scope.terminate_execution();
742761
return false;
743762
},
@@ -778,13 +797,13 @@ fn dispatch_event_loop_frame(
778797
let (result, error) = if status == 1 {
779798
(None, Some(String::from_utf8_lossy(&payload).to_string()))
780799
} else if !payload.is_empty() {
781-
// status=0: V8-serialized, status=2: raw binary (Uint8Array)
800+
// V8-serialized or raw binary
782801
(Some(payload), None)
783802
} else {
784803
(None, None)
785804
};
786805
let _ = crate::bridge::resolve_pending_promise(scope, pending, call_id, result, error);
787-
// Microtasks already flushed in resolve_pending_promise
806+
scope.perform_microtask_checkpoint();
788807
true
789808
}
790809
BinaryFrame::StreamEvent {
@@ -846,6 +865,10 @@ impl ChannelResponseReceiver {
846865
}
847866

848867
impl crate::host_call::ResponseReceiver for ChannelResponseReceiver {
868+
fn defer(&self, frame: BinaryFrame) {
869+
self.deferred.lock().unwrap().push_back(frame);
870+
}
871+
849872
fn recv_response(&self) -> Result<BinaryFrame, String> {
850873
loop {
851874
// Wait for next command, with optional abort monitoring

packages/v8/src/runtime.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,14 @@ export async function createV8Runtime(
120120

121121
child.on("exit", (code, signal) => {
122122
processAlive = false;
123+
const stderrSuffix = stderrBuf ? `\nstderr: ${stderrBuf}` : "";
123124
if (code !== 0 && code !== null) {
124125
exitError = new Error(
125-
`V8 runtime process exited with code ${code}`,
126+
`V8 runtime process exited with code ${code}${stderrSuffix}`,
126127
);
127128
} else if (signal) {
128129
exitError = new Error(
129-
`V8 runtime process killed by signal ${signal}`,
130+
`V8 runtime process killed by signal ${signal}${stderrSuffix}`,
130131
);
131132
}
132133

@@ -184,8 +185,9 @@ export async function createV8Runtime(
184185
// Reject all pending executions — the Rust process may have
185186
// deadlocked without exiting, so we can't rely on the 'exit'
186187
// event alone.
188+
const stderrInfo = stderrBuf ? `\nstderr: ${stderrBuf}` : "";
187189
rejectPendingSessions(
188-
new Error("IPC connection closed"),
190+
new Error(`IPC connection closed${stderrInfo}`),
189191
);
190192
},
191193
onError: (err) => {

0 commit comments

Comments
 (0)