diff --git a/moli-protocol-server/src/cdp_scheduler/runtime_dispatch.rs b/moli-protocol-server/src/cdp_scheduler/runtime_dispatch.rs index 52a445944..fa078314e 100644 --- a/moli-protocol-server/src/cdp_scheduler/runtime_dispatch.rs +++ b/moli-protocol-server/src/cdp_scheduler/runtime_dispatch.rs @@ -42,6 +42,7 @@ pub(super) fn devtools_command_uses_interleaved_runtime_dispatch( DevToolsCommand::GetRealms(_) | DevToolsCommand::EvaluateScript(_) | DevToolsCommand::CallFunction(_) + | DevToolsCommand::TerminateExecution(_) | DevToolsCommand::LocateNodes(_) | DevToolsCommand::ReleaseObjects(_) ) diff --git a/moli-protocol-server/src/protocol_server/tests/classic.rs b/moli-protocol-server/src/protocol_server/tests/classic.rs index 1946aedb4..b8dc57f83 100644 --- a/moli-protocol-server/src/protocol_server/tests/classic.rs +++ b/moli-protocol-server/src/protocol_server/tests/classic.rs @@ -10962,6 +10962,69 @@ async fn webdriver_classic_execute_sync_honors_script_timeout() { .await; } +#[tokio::test] +async fn webdriver_classic_execute_sync_timeout_interrupts_non_yielding_script() { + let app = build_router(test_state()); + + let session = classic_request_json(app.clone(), Method::POST, "/session").await; + let session_id = session["value"]["sessionId"] + .as_str() + .expect("classic session id"); + + let set_timeouts = classic_request_json_with_body( + app.clone(), + Method::POST, + &format!("/session/{session_id}/timeouts"), + json!({ "script": 100 }), + ) + .await; + assert_eq!(set_timeouts, json!({ "value": null })); + + let (timeout_status, timeout_response) = tokio::time::timeout( + Duration::from_secs(10), + classic_request_status_and_json_with_body( + app.clone(), + Method::POST, + &format!("/session/{session_id}/execute/sync"), + json!({ + "script": "for (;;) {}", + "args": [] + }), + ), + ) + .await + .expect("non-yielding script timeout must interrupt V8 and return"); + assert_eq!(timeout_status, StatusCode::REQUEST_TIMEOUT); + assert_eq!(timeout_response["value"]["error"], json!("script timeout")); + + let reset_timeouts = classic_request_json_with_body( + app.clone(), + Method::POST, + &format!("/session/{session_id}/timeouts"), + json!({ "script": 1000 }), + ) + .await; + assert_eq!(reset_timeouts, json!({ "value": null })); + + let recovered = tokio::time::timeout( + Duration::from_secs(5), + classic_request_json_with_body( + app.clone(), + Method::POST, + &format!("/session/{session_id}/execute/sync"), + json!({ + "script": "return 42;", + "args": [] + }), + ), + ) + .await + .expect("renderer must accept another script after timeout termination"); + assert_eq!(recovered, json!({ "value": 42 })); + + let _ = classic_request_json(app, Method::DELETE, &format!("/session/{session_id}")).await; +} + #[tokio::test] async fn webdriver_classic_execute_async_honors_script_timeout() { let app = build_router(test_state()); diff --git a/moli-protocol-server/src/protocol_server/webdriver_classic.rs b/moli-protocol-server/src/protocol_server/webdriver_classic.rs index 1eac73950..95dfe87d4 100644 --- a/moli-protocol-server/src/protocol_server/webdriver_classic.rs +++ b/moli-protocol-server/src/protocol_server/webdriver_classic.rs @@ -2523,7 +2523,7 @@ pub(super) async fn webdriver_classic_execute_sync( apply_classic_script_argument_handles(&mut command, &script_argument_handles); let result = binding .runtime - .execute_inner(command, binding.timeouts.script.map(Duration::from_millis)) + .execute_script(command, binding.timeouts.script.map(Duration::from_millis)) .await; release_classic_remote_objects( &binding, @@ -2594,7 +2594,7 @@ pub(super) async fn webdriver_classic_execute_async( apply_classic_script_argument_handles(&mut command, &script_argument_handles); let result = binding .runtime - .execute_inner(command, binding.timeouts.script.map(Duration::from_millis)) + .execute_script(command, binding.timeouts.script.map(Duration::from_millis)) .await; release_classic_remote_objects( &binding, @@ -3866,7 +3866,7 @@ pub(super) async fn webdriver_classic_get_element_property( apply_classic_script_argument_handles(&mut command, &script_argument_handles); let result = binding .runtime - .execute_inner(command, binding.timeouts.script.map(Duration::from_millis)) + .execute_script(command, binding.timeouts.script.map(Duration::from_millis)) .await; release_classic_remote_objects( &binding, diff --git a/moli-protocol-server/src/protocol_server/webdriver_classic/state.rs b/moli-protocol-server/src/protocol_server/webdriver_classic/state.rs index bafea6821..d98410aeb 100644 --- a/moli-protocol-server/src/protocol_server/webdriver_classic/state.rs +++ b/moli-protocol-server/src/protocol_server/webdriver_classic/state.rs @@ -15,7 +15,7 @@ use moli_protocol::{ DevToolsCommand, DevToolsCommandContext, DevToolsCommandResult, DevToolsDomNodeReference, DevToolsError, DevToolsErrorKind, DevToolsFrameId, DevToolsGetFrameOwnerCommand, DevToolsGetFrameOwnerResult, DevToolsGetFrameTreeCommand, DevToolsProtocol, - DevToolsSessionId, DevToolsTargetId, + DevToolsSessionId, DevToolsTargetId, DevToolsTerminateExecutionCommand, }, }; use moli_protocol_webdriver_classic::{ @@ -38,6 +38,8 @@ use super::super::webdriver_bidi::{ }; use super::super::{CookieProfileCommit, protocol_local_executor::spawn_protocol_local_task}; +const CLASSIC_SCRIPT_TERMINATION_TIMEOUT: Duration = Duration::from_secs(5); + #[derive(Debug, Clone, Default)] pub(in crate::protocol_server) struct SharedClassicSessionRegistry { inner: Arc>, @@ -536,7 +538,17 @@ impl ClassicSessionRuntimeHandle { command: DevToolsCommand, timeout: Option, ) -> Result { - self.execute_with_options(command, timeout, None).await + self.execute_with_options(command, timeout, None, false) + .await + } + + pub(super) async fn execute_script( + &self, + command: DevToolsCommand, + timeout: Option, + ) -> Result { + self.execute_with_options(command, timeout, None, true) + .await } pub(super) async fn execute_with_pending_navigation_wait( @@ -545,7 +557,7 @@ impl ClassicSessionRuntimeHandle { timeout: Option, pending_navigation_timeout: Option, ) -> Result { - self.execute_with_options(command, timeout, pending_navigation_timeout) + self.execute_with_options(command, timeout, pending_navigation_timeout, false) .await } @@ -582,6 +594,7 @@ impl ClassicSessionRuntimeHandle { command: DevToolsCommand, timeout: Option, pending_navigation_timeout: Option, + terminate_execution_on_timeout: bool, ) -> Result { let (response_tx, response_rx) = oneshot::channel(); self.tx @@ -589,6 +602,7 @@ impl ClassicSessionRuntimeHandle { command: Box::new(command), timeout, pending_navigation_timeout, + terminate_execution_on_timeout, response_tx, }) .map_err(|_| { @@ -793,6 +807,7 @@ enum ClassicSessionRuntimeRequest { command: Box, timeout: Option, pending_navigation_timeout: Option, + terminate_execution_on_timeout: bool, response_tx: oneshot::Sender>, }, WaitForDocumentLifecycle { @@ -880,9 +895,11 @@ async fn handle_classic_session_runtime_request( command, timeout, pending_navigation_timeout, + terminate_execution_on_timeout, response_tx, } => { - let execution = execute_classic_devtools_command_with_pending_navigation_retry( + let termination_context = command.context().clone(); + let mut execution = execute_classic_devtools_command_with_pending_navigation_retry( scheduler, receivers, *command, @@ -890,6 +907,34 @@ async fn handle_classic_session_runtime_request( pending_navigation_timeout, ) .await; + if terminate_execution_on_timeout + && matches!( + execution.result, + Err(ref error) if error.kind == DevToolsErrorKind::Timeout + ) + { + // Finish the IO-side termination before the HTTP handler + // releases argument handles or admits the next Classic + // command on this session. + let termination = execute_classic_devtools_command_once( + scheduler, + receivers, + DevToolsCommand::TerminateExecution(DevToolsTerminateExecutionCommand { + context: termination_context, + }), + Some(CLASSIC_SCRIPT_TERMINATION_TIMEOUT), + ) + .await; + if let Err(error) = &termination.result { + tracing::warn!( + ?error, + "failed to terminate timed-out WebDriver Classic script execution" + ); + } + execution + .protocol_output + .append(termination.protocol_output); + } let result = execution.result; let keep_attached = if let Some(attached) = attached_bidi.as_mut() { attached diff --git a/moli-protocol/src/conn/devtools_command.rs b/moli-protocol/src/conn/devtools_command.rs index 022240000..16e4bd324 100644 --- a/moli-protocol/src/conn/devtools_command.rs +++ b/moli-protocol/src/conn/devtools_command.rs @@ -256,6 +256,7 @@ impl CdpConnection { command @ (DevToolsCommand::GetRealms(_) | DevToolsCommand::EvaluateScript(_) | DevToolsCommand::CallFunction(_) + | DevToolsCommand::TerminateExecution(_) | DevToolsCommand::LocateNodes(_) | DevToolsCommand::ReleaseObjects(_)) => { let output = Box::pin( diff --git a/moli-protocol/src/devtools_runtime.rs b/moli-protocol/src/devtools_runtime.rs index b765279f1..53da2d0d7 100644 --- a/moli-protocol/src/devtools_runtime.rs +++ b/moli-protocol/src/devtools_runtime.rs @@ -216,6 +216,7 @@ pub enum DevToolsCommand { GetRealms(DevToolsGetRealmsCommand), EvaluateScript(DevToolsEvaluateScriptCommand), CallFunction(DevToolsCallFunctionCommand), + TerminateExecution(DevToolsTerminateExecutionCommand), ReleaseObjects(DevToolsReleaseObjectsCommand), CreateTarget(DevToolsCreateTargetCommand), CloseTarget(DevToolsCloseTargetCommand), @@ -301,6 +302,7 @@ impl DevToolsCommand { DevToolsCommand::GetRealms(command) => &command.context, DevToolsCommand::EvaluateScript(command) => &command.context, DevToolsCommand::CallFunction(command) => &command.context, + DevToolsCommand::TerminateExecution(command) => &command.context, DevToolsCommand::ReleaseObjects(command) => &command.context, DevToolsCommand::CreateTarget(command) => &command.context, DevToolsCommand::CloseTarget(command) => &command.context, @@ -468,6 +470,11 @@ pub struct DevToolsCallFunctionCommand { pub materialize_bidi_script_result: bool, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DevToolsTerminateExecutionCommand { + pub context: DevToolsCommandContext, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct DevToolsSerializationOptions { pub max_object_depth: Option, diff --git a/moli-protocol/src/domains/runtime/dispatcher.rs b/moli-protocol/src/domains/runtime/dispatcher.rs index ee342a309..6eda9354d 100644 --- a/moli-protocol/src/domains/runtime/dispatcher.rs +++ b/moli-protocol/src/domains/runtime/dispatcher.rs @@ -121,12 +121,19 @@ struct DevToolsRuntimeTarget { struct DevToolsRuntimeCommandDispatchState { internal_command_id: u64, command_context: DevToolsCommandContext, + result_kind: DevToolsRuntimeCommandResultKind, result_ownership: DevToolsResultOwnership, serialization_options: Option, target: DevToolsRuntimeTarget, target_realm: Option, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum DevToolsRuntimeCommandResultKind { + Script, + Empty, +} + pub struct PendingDevToolsRuntimeCommandDispatch { state: DevToolsRuntimeCommandDispatchState, pending: PendingRuntimeCommandDispatch, @@ -2143,13 +2150,22 @@ pub(crate) async fn execute_devtools_runtime_command_async_with_protocol_events( if let DevToolsCommand::LocateNodes(command) = command { return execute_devtools_locate_nodes_command_async(conn, command).await; } + let result_kind = devtools_runtime_command_result_kind(&command); let result_ownership = devtools_runtime_result_ownership(&command); let serialization_options = devtools_runtime_serialization_options(&command); let target = match devtools_runtime_target_async(conn, &command).await { Ok(target) => target, Err(error) => return DevToolsCommandExecutionOutput::new(Err(error)), }; - let target_realm = devtools_realm_id_for_runtime_target_async(conn, &target).await; + // Control commands must reach their Inspector route without first asking + // the Page owner for realm inventory. That owner can be the JavaScript + // execution this command exists to interrupt. + let target_realm = match result_kind { + DevToolsRuntimeCommandResultKind::Script => { + devtools_realm_id_for_runtime_target_async(conn, &target).await + } + DevToolsRuntimeCommandResultKind::Empty => None, + }; let mut route_scope = conn.scoped_none_session_owner_route_override(target.route.clone()); if let DevToolsCommand::CallFunction(call_function) = &mut command && matches!( @@ -2198,6 +2214,13 @@ pub(crate) async fn execute_devtools_runtime_command_async_with_protocol_events( renderer_output_predecessor, ); }; + if result_kind == DevToolsRuntimeCommandResultKind::Empty { + return DevToolsCommandExecutionOutput::from_parts( + devtools_empty_result_from_response(response), + protocol_events, + renderer_output_predecessor, + ); + } let mut result = match devtools_script_result_from_response( response, result_ownership, @@ -2316,6 +2339,7 @@ impl CdpConnection { .await; } + let result_kind = devtools_runtime_command_result_kind(&command); let result_ownership = devtools_runtime_result_ownership(&command); let serialization_options = devtools_runtime_serialization_options(&command); let target = match devtools_runtime_target_async(self, &command).await { @@ -2331,7 +2355,15 @@ impl CdpConnection { .await; } }; - let target_realm = devtools_realm_id_for_runtime_target_async(self, &target).await; + // Keep the interrupt path free of Page-owner realm lookups. In + // particular, Runtime.terminateExecution must be able to enter its IO + // envelope while a MainThread script is not yielding. + let target_realm = match result_kind { + DevToolsRuntimeCommandResultKind::Script => { + devtools_realm_id_for_runtime_target_async(self, &target).await + } + DevToolsRuntimeCommandResultKind::Empty => None, + }; let mut route_scope = self.scoped_none_session_owner_route_override(target.route.clone()); if let DevToolsCommand::CallFunction(call_function) = &mut command && matches!( @@ -2377,6 +2409,7 @@ impl CdpConnection { let state = DevToolsRuntimeCommandDispatchState { internal_command_id, command_context, + result_kind, result_ownership, serialization_options, target: target.clone(), @@ -2456,6 +2489,16 @@ impl CdpConnection { ) .await; }; + if state.result_kind == DevToolsRuntimeCommandResultKind::Empty { + return self + .complete_devtools_runtime_direct_result( + state.command_context, + devtools_empty_result_from_response(response), + protocol_events, + renderer_output_predecessor, + ) + .await; + } let mut result = match devtools_script_result_from_response( response, state.result_ownership, @@ -3493,6 +3536,15 @@ fn devtools_runtime_result_ownership(command: &DevToolsCommand) -> DevToolsResul } } +fn devtools_runtime_command_result_kind( + command: &DevToolsCommand, +) -> DevToolsRuntimeCommandResultKind { + match command { + DevToolsCommand::TerminateExecution(_) => DevToolsRuntimeCommandResultKind::Empty, + _ => DevToolsRuntimeCommandResultKind::Script, + } +} + fn devtools_runtime_serialization_options( command: &DevToolsCommand, ) -> Option { @@ -3547,6 +3599,13 @@ async fn devtools_runtime_target_async( conn: &mut CdpConnection, command: &DevToolsCommand, ) -> Result { + if let DevToolsCommand::TerminateExecution(command) = command { + let target_id = + command.context.target_id.as_ref().ok_or_else(|| { + DevToolsError::new(DevToolsErrorKind::NoSuchTarget, "NoSuchTarget") + })?; + return devtools_runtime_control_target(conn, target_id); + } let (target_id, realm_id, world_name) = match command { DevToolsCommand::EvaluateScript(command) => ( command.context.target_id.as_ref(), @@ -3578,6 +3637,23 @@ async fn devtools_runtime_target_async( )) } +fn devtools_runtime_control_target( + conn: &CdpConnection, + target_id: &DevToolsTargetId, +) -> Result { + // Do not fall back to realm discovery here: it is a MainThread operation + // and would put the escape hatch behind the work it needs to interrupt. + let route = conn + .target_session_route_for_target_id(target_id.as_str()) + .or_else(|| conn.target_session_route_for_child_frame_id(target_id.as_str())) + .ok_or_else(|| DevToolsError::new(DevToolsErrorKind::NoSuchTarget, "NoSuchTarget"))?; + Ok(DevToolsRuntimeTarget { + route, + execution_context_id: None, + window_context_id: Some(target_id.clone()), + }) +} + async fn devtools_runtime_context_target_async( conn: &mut CdpConnection, target_id: &DevToolsTargetId, @@ -4574,6 +4650,30 @@ async fn start_protocol_neutral_runtime_command( )), } } + DevToolsCommand::TerminateExecution(_) => { + let json = runtime_inspector_command_json( + internal_command_id, + "Runtime.terminateExecution", + &json!({}), + ); + let parsed = match parse_synthesized_runtime_command(json) { + Ok(command) => command, + Err(message) => { + return RuntimeCommandTaskStep::Complete(runtime_inspector_error_plan( + Some(internal_command_id), + message, + )); + } + }; + let cmd = Cmd::from_parsed(&parsed) + .expect("synthesized Runtime command must contain a domain separator"); + let MainRuntimeCommand::Inspector(command) = + MainRuntimeCommand::classify(RuntimeAction::TerminateExecution) + else { + unreachable!("Runtime.terminateExecution must use an Inspector command route") + }; + start_main_runtime_inspector_command(route_scope.conn_mut(), &cmd, command) + } _ => RuntimeCommandTaskStep::Complete(CommandOutputPlan::from_devtools_error( DevToolsError::new(DevToolsErrorKind::Unsupported, "UnsupportedDevToolsCommand"), )), @@ -5516,6 +5616,15 @@ fn devtools_script_result_from_response( ))) } +fn devtools_empty_result_from_response( + response: Value, +) -> Result { + if let Some(error) = response.get("error") { + return Err(devtools_error_from_cdp_error_value(error)); + } + Ok(DevToolsCommandResult::Empty) +} + fn validate_protocol_neutral_runtime_handle_realms( conn: &CdpConnection, command: &DevToolsCommand,