diff --git a/crates/ark/src/console.rs b/crates/ark/src/console.rs index 7e61981b5f..3eb02c6571 100644 --- a/crates/ark/src/console.rs +++ b/crates/ark/src/console.rs @@ -92,7 +92,6 @@ use harp::utils::r_typeof; use harp::CONSOLE_THREAD_ID; use libr::R_BaseNamespace; use libr::R_ProcessEvents; -use libr::R_RunPendingFinalizers; use libr::Rf_ScalarInteger; use libr::Rf_error; use libr::Rf_findVarInFrame; @@ -128,8 +127,7 @@ use console_filter::ConsoleFilter; pub use console_repl::catching_panics; pub(crate) use console_repl::console_inputs; pub(crate) use console_repl::r_busy; -#[cfg(unix)] -pub(crate) use console_repl::r_polled_events; +pub(crate) use console_repl::r_process_events; pub(crate) use console_repl::r_read_console; pub(crate) use console_repl::r_show_message; pub(crate) use console_repl::r_suicide; @@ -235,7 +233,6 @@ pub struct Console { autoprint_output: String, /// Channel to send and receive tasks from `QueuedRTask`s - tasks_interrupt_rx: Receiver, tasks_idle_rx: Receiver, tasks_idle_any_rx: Receiver, pending_futures: HashMap, RTaskStartInfo, Option)>, diff --git a/crates/ark/src/console/console_filter.rs b/crates/ark/src/console/console_filter.rs index 71fa2da47e..084dc77d2a 100644 --- a/crates/ark/src/console/console_filter.rs +++ b/crates/ark/src/console/console_filter.rs @@ -263,7 +263,6 @@ impl ConsoleFilter { /// Check for timeout and handle state transitions. /// Timeout means we didn't reach ReadConsole to confirm debug output, /// so we emit the accumulated content back to the user. - #[cfg(any(unix, test))] pub(super) fn check_timeout(&mut self) -> Option { self.drain_on_timeout() } diff --git a/crates/ark/src/console/console_repl.rs b/crates/ark/src/console/console_repl.rs index 6897526aad..1ed676ed04 100644 --- a/crates/ark/src/console/console_repl.rs +++ b/crates/ark/src/console/console_repl.rs @@ -375,11 +375,10 @@ impl Console { }; } - let (tasks_interrupt_rx, tasks_idle_rx, tasks_idle_any_rx) = r_task::take_receivers(); + let (tasks_idle_rx, tasks_idle_any_rx) = r_task::take_receivers(); CONSOLE.set(UnsafeCell::new(Console::new( r_home, - tasks_interrupt_rx, tasks_idle_rx, tasks_idle_any_rx, comm_event_tx, @@ -506,9 +505,12 @@ impl Console { // integration tests by spawning an async task. The deadlock is caused // by the `block_on()` behaviour in // https://github.com/posit-dev/ark/blob/bd827e73/crates/ark/src/r_task.rs#L261. - r_task::spawn(RTask::interrupt({ + // + // Use `idle_any_prompt` so DAP breakpoint invalidation still fires when + // the LSP signals a document change while R is paused at `browser()`. + r_task::spawn(RTask::idle_any_prompt({ let dap_clone = console.debug_dap.clone(); - async move || { + async move |_| { Console::process_console_notifications(console_notification_rx, dap_clone).await } })); @@ -614,7 +616,6 @@ impl Console { fn new( r_home: PathBuf, - tasks_interrupt_rx: Receiver, tasks_idle_rx: Receiver, tasks_idle_any_rx: Receiver, comm_event_tx: Sender, @@ -649,7 +650,6 @@ impl Console { debug_dap: dap, debug_is_debugging: false, debug_stopped_reason: None, - tasks_interrupt_rx, tasks_idle_rx, tasks_idle_any_rx, pending_futures: HashMap::new(), @@ -757,7 +757,6 @@ impl Console { } init.set(true); - let (_, tasks_interrupt_rx) = crossbeam::channel::unbounded(); let (_, tasks_idle_rx) = crossbeam::channel::unbounded(); let (_, tasks_idle_any_rx) = crossbeam::channel::unbounded(); let (comm_event_tx, _) = crossbeam::channel::unbounded(); @@ -770,7 +769,6 @@ impl Console { CONSOLE.set(UnsafeCell::new(Console::new( PathBuf::new(), - tasks_interrupt_rx, tasks_idle_rx, tasks_idle_any_rx, comm_event_tx, @@ -853,7 +851,8 @@ impl Console { self.active_request.as_ref().map(|req| &req.request) } - // Async messages for the Console. Processed at interrupt time. + // Async messages for the Console. Polled at any idle prompt (top-level or + // browser) so DAP doc-change handling continues during debug sessions. async fn process_console_notifications( mut console_notification_rx: AsyncUnboundedReceiver, dap: Arc>, @@ -1070,9 +1069,10 @@ impl Console { /// This handles events for: /// - Reception of either input replies or execute requests (as determined /// by `wait_for`) - /// - Idle-time and interrupt-time tasks + /// - Idle-time tasks /// - Requests from the frontend (currently only used for establishing UI comm) - /// - R's polled events + /// - R's activity handlers + /// - R's `R_ProcessEvents()` fn run_event_loop( &mut self, info: &PromptInfo, @@ -1086,15 +1086,19 @@ impl Console { let r_request_rx = self.r_request_rx.clone(); let stdin_reply_rx = self.stdin_reply_rx.clone(); let kernel_request_rx = self.kernel_request_rx.clone(); - let tasks_interrupt_rx = self.tasks_interrupt_rx.clone(); let tasks_idle_rx = self.tasks_idle_rx.clone(); let tasks_idle_any_rx = self.tasks_idle_any_rx.clone(); - // Process R's polled events regularly while waiting for console input. - // We used to poll every 200ms but that lead to visible delays for the - // processing of plot events, it also slowed down callbacks from the later + // Run activity handlers regularly while waiting for console input. + // We used to poll every 200ms but that slowed down callbacks from the later // package. 50ms seems to be more in line with RStudio (posit-dev/positron#7235). - let polled_events_rx = crossbeam::channel::tick(Duration::from_millis(50)); + let activity_handlers_rx = crossbeam::channel::tick(Duration::from_millis(50)); + + // Run `R_ProcessEvents()` regularly out of good faith. We don't think this + // actually does all that much on the R side, and our use case to flush + // `debug_filter` isn't that critical, so we don't run them as often as activity + // handlers. + let process_events_rx = crossbeam::channel::tick(Duration::from_millis(200)); // This is the main kind of message from the frontend that we are // expecting. We either wait for `input_reply` messages on StdIn, or for @@ -1110,8 +1114,8 @@ impl Console { }; let kernel_request_index = select.recv(&kernel_request_rx); - let tasks_interrupt_index = select.recv(&tasks_interrupt_rx); - let polled_events_index = select.recv(&polled_events_rx); + let activity_handlers_index = select.recv(&activity_handlers_rx); + let process_events_index = select.recv(&process_events_rx); // Only process idle at top level. We currently don't want idle tasks // (e.g. for srcref generation) to run when the call stack is not empty. @@ -1132,12 +1136,13 @@ impl Console { loop { // If an interrupt was signaled and we are waiting for user // input (readline, or browser-as-stdin in notebook mode), we - // need to propagate the interrupt to the R stack. This needs - // to happen before `process_idle_events()`, particularly on - // Windows, because it calls `R_ProcessEvents()`, which checks - // and resets `UserBreak`, but won't actually fire the - // interrupt b/c we have them disabled, so it would end up - // swallowing the user interrupt request. + // need to propagate the interrupt to the R stack. + // + // This needs to happen before we `select()`, particularly for + // `process_events()` on Windows. `R_ProcessEvents()` will reset `UserBreak` + // there and call `onintr()`, but `onintr()` won't actually fire the interrupt + // b/c we have them disabled while inside `run_event_loop()`, so it would end + // up swallowing the user interrupt request. if matches!(wait_for, WaitFor::InputReply) && interrupts_pending() { return ConsoleResult::Interrupt; } @@ -1207,12 +1212,6 @@ impl Console { self.handle_kernel_request(req); }, - // An interrupt task woke us up - i if i == tasks_interrupt_index => { - let task = oper.recv(&tasks_interrupt_rx).unwrap(); - self.handle_task_interrupt(task); - }, - // An idle task woke us up i if Some(i) == tasks_idle_index => { let task = oper.recv(&tasks_idle_rx).unwrap(); @@ -1225,10 +1224,16 @@ impl Console { self.handle_task(task); }, - // It's time to run R's polled events - i if i == polled_events_index => { - let _ = oper.recv(&polled_events_rx).unwrap(); - Self::process_idle_events(); + // It's time to run activity handlers + i if i == activity_handlers_index => { + let _ = oper.recv(&activity_handlers_rx).unwrap(); + Self::run_activity_handlers(); + }, + + // It's time to run R's `R_ProcessEvents()` + i if i == process_events_index => { + let _ = oper.recv(&process_events_rx).unwrap(); + Self::run_process_events(); }, i => log::error!("Unexpected index in Select: {i}"), @@ -1948,36 +1953,30 @@ impl Console { } } - /// Handle a task at interrupt time. - /// - /// Wrapper around `handle_task()` that does some extra logging to record - /// how long a task waited before being picked up by the R or ReadConsole - /// event loop. + /// Handle a task /// - /// Since tasks running during interrupt checks block the R thread while - /// they are running, they should return very quickly. The log message helps - /// monitor excessively long-running tasks. - fn handle_task_interrupt(&mut self, mut task: QueuedRTask) { - if let Some(start_info) = task.start_info_mut() { - // Log excessive waiting before starting task - if start_info.start_time.elapsed() > std::time::Duration::from_millis(50) { - start_info.span.in_scope(|| { - tracing::info!( - "{} milliseconds wait before running task.", - start_info.start_time.elapsed().as_millis() - ) - }); - } + /// The log message helps monitor excessively long-running tasks. + fn handle_task(&mut self, mut task: QueuedRTask) { + // For Sync tasks (i.e. only `r_task()`s), we want to log excessive waiting, + // because we are blocking the calling thread + if matches!(task, QueuedRTask::Sync(_)) { + if let Some(start_info) = task.start_info_mut() { + if start_info.start_time.elapsed() > std::time::Duration::from_millis(50) { + start_info.span.in_scope(|| { + tracing::info!( + "{} milliseconds wait before running task.", + start_info.start_time.elapsed().as_millis() + ) + }); + } - // Reset timer, next time we'll log how long the task took - start_info.start_time = std::time::Instant::now(); + // Reset timer, next time we'll log how long the task took + start_info.start_time = std::time::Instant::now(); + } } - let finished_task_info = self.handle_task(task); + let finished_task_info = self.handle_task_inner(task); - // We only log long task durations in the interrupt case since we expect - // idle tasks to take longer. Use the tracing profiler to monitor the - // duration of idle tasks. if let Some(info) = finished_task_info { if info.elapsed() > std::time::Duration::from_millis(50) { info.span.in_scope(|| { @@ -1988,7 +1987,7 @@ impl Console { } /// Returns start information when the task has been completed - fn handle_task(&mut self, task: QueuedRTask) -> Option { + fn handle_task_inner(&mut self, task: QueuedRTask) -> Option { // Background tasks can't take any user input, so we set R_Interactive // to 0 to prevent `readline()` from blocking the task. let _interactive = harp::raii::RLocalInteractive::new(false); @@ -2448,24 +2447,51 @@ impl Console { } } - /// Invoked by the R event loop - #[cfg(unix)] - fn polled_events(&mut self) { - // Don't process tasks until R is fully initialized - if !Self::is_initialized() { - if !self.tasks_interrupt_rx.is_empty() { - log::trace!("Delaying execution of interrupt task as R isn't initialized yet"); - } - return; - } + fn run_activity_handlers() { + crate::sys::console::run_activity_handlers(); + } - // Skip running tasks if we don't have 128KB of stack space available. - // This is 1/8th of the typical Windows stack space (1MB, whereas macOS - // and Linux have 8MB). - if harp::exec::r_check_stack(Some(128 * 1024)).is_err() { - return; - } + /// Invoke `R_ProcessEvents()` + /// + /// We call this out of good faith at regular intervals while idling in the event + /// loop, but we don't think it actually does very much on the R side. It is what + /// ends up calling our `process_events()` hook, which drains `debug_filter` during + /// long computations, but that is a non-critical use case. + /// + /// Also, R itself will call `R_ProcessEvents()` at regular times, like via + /// `R_CheckUserInterrupt()`. + /// + /// Unix : + /// - Calls `ptr_R_ProcessEvents()`, our `process_events()` + /// - Calls `R_PolledEvents()`, a no-op since we don't set it + /// - Calls `R_CheckTimeLimits()` + /// + /// Windows : + /// - Calls graphapp's `doevent()` (but we are unsure if you can even use graphapp + /// in Ark) + /// - Calls `R_CheckTimeLimits()` + /// - If `UserBreak=true`, sets it to `false` and calls `onintr()`. Never the case + /// for us, since `run_event_loop()` always sets `set_interrupts_pending(false)`. + /// - Calls `ptr_R_ProcessEvents()`, i.e. `Rp->Callback`, i.e. our `process_events()` + /// - Calls `R_Tcl_do` (but we are unsure if you can even use tcktk in Ark) + fn run_process_events() { + unsafe { R_ProcessEvents() }; + } + /// Hook invoked by `R_ProcessEvents()` + /// + /// This hook is run at regular intervals in `run_event_loop()` via + /// `run_process_events()` calling `R_ProcessEvents()`, which ends up calling us via + /// `ptr_R_ProcessEvents()`. + /// + /// It is also called at interrupt time via `R_CheckUserInterrupt()` calling + /// `R_ProcessEvents()`, but this happens very irregularly and is dependent on both + /// base R and other R packages checking this, so we should never rely on that. + /// + /// We should only use this for non-critical side effects / clean up. And ideally it + /// should not run any R code, because running R code at interrupt time is generally + /// unsafe. + fn process_events(&mut self) { // Check stream filter timeout to handle long computations between // WriteConsole calls. Timeout means we didn't reach ReadConsole to // confirm debug output within a reasonable amount of time, so @@ -2475,33 +2501,6 @@ impl Console { if let Some(text) = self.debug_filter.check_timeout() { self.emit_stdout(text); } - - // Coalesce up to three concurrent tasks in case the R event loop is - // slowed down - for _ in 0..3 { - if let Ok(task) = self.tasks_interrupt_rx.try_recv() { - self.handle_task_interrupt(task); - } else { - break; - } - } - } - - fn process_idle_events() { - // Process regular R events. We're normally running with polled - // events disabled so that won't run here. We also run with - // interrupts disabled, so on Windows those won't get run here - // either (i.e. if `UserBreak` is set), but it will reset `UserBreak` - // so we need to ensure we handle interrupts right before calling - // this. - unsafe { R_ProcessEvents() }; - - crate::sys::console::run_activity_handlers(); - - // Run pending finalizers. We need to do this eagerly as otherwise finalizers - // might end up being executed on the LSP thread. - // https://github.com/rstudio/positron/issues/431 - unsafe { R_RunPendingFinalizers() }; } pub(super) fn eval_env(&self) -> RObject { @@ -2873,11 +2872,10 @@ pub extern "C-unwind" fn r_suicide(buf: *const c_char) { panic!("Suicide: {}", msg.to_str().unwrap()); } -#[cfg(unix)] #[cfg_attr(not(test), no_mangle)] -pub unsafe extern "C-unwind" fn r_polled_events() { - if let Err(err) = r_sandbox(|| Console::get_mut().polled_events()) { - panic!("Unexpected longjump while polling events: {err:?}"); +pub unsafe extern "C-unwind" fn r_process_events() { + if let Err(err) = r_sandbox(|| Console::get_mut().process_events()) { + panic!("Unexpected longjump while processing events: {err:?}"); }; } diff --git a/crates/ark/src/lsp/backend.rs b/crates/ark/src/lsp/backend.rs index 9d4feb6417..ee877032eb 100644 --- a/crates/ark/src/lsp/backend.rs +++ b/crates/ark/src/lsp/backend.rs @@ -10,6 +10,7 @@ use std::path::PathBuf; use std::sync::atomic::Ordering; use std::sync::Arc; +use std::time::Duration; use amalthea::comm::server_comm::ServerStartMessage; use amalthea::comm::server_comm::ServerStartedMessage; @@ -85,7 +86,7 @@ macro_rules! cast_response { }, RequestResponse::Crashed(err) => { // Notify user that the LSP has crashed and is no longer active - report_crash(); + report_crash($self.client()).await; // The backtrace is reported via `err` and eventually shows up // in the LSP logs on the client side @@ -99,21 +100,34 @@ macro_rules! cast_response { }}; } -fn report_crash() { +/// Send via `request::ShowMessageRequest` not `notification::ShowMessage` so that we can +/// ensure that the message has been received on the frontend side. We are about to shut +/// the LSP down, and sending out a fire-and-forget notification often won't get sent out +/// before shutdown occurs. The request returns control to us when the user acknowledges +/// the message. It doesn't matter if that takes awhile because we shut down right after, +/// and we've already flipped the `LSP_HAS_CRASHED` global flag, but we do bound it with +/// a 5 second timeout just in case the user ignores the message entirely, so we can still +/// shutdown. +async fn report_crash(client: &Client) { let user_message = concat!( "The R language server has crashed and has been disabled. ", "Smart features such as completions will no longer work in this session. ", "Please report this crash to https://github.com/posit-dev/positron/issues ", "with full logs (see https://positron.posit.co/troubleshooting.html#python-and-r-logs)." ); - - // NOTE: This is a legit use of interrupt-time task. No R access here, and - // we need to go through Console since it owns the UI comm. - r_task(|| { - if let Some(ui) = Console::get().ui_comm() { - ui.show_message(String::from(user_message)); - } + let request = client.send_request::(ShowMessageRequestParams { + typ: MessageType::ERROR, + message: String::from(user_message), + actions: None, }); + match tokio::time::timeout(Duration::from_secs(5), request).await { + Ok(result) => { + result.log_err(); + }, + Err(_) => { + log::warn!("Timed out waiting for frontend to acknowledge LSP crash notification"); + }, + } } #[derive(Debug)] @@ -228,6 +242,9 @@ struct Backend { /// Channel for communication with the main loop. events_tx: TokioUnboundedSender, + /// Copy of the Client, for reporting crash messages. + client: Client, + /// Handle to main loop. Drop it to cancel the loop, all associated tasks, /// and drop all owned state. _main_loop: tokio::task::JoinSet<()>, @@ -256,6 +273,10 @@ impl Backend { .send(Event::Lsp(LspMessage::Notification(notif))) .unwrap(); } + + fn client(&self) -> &Client { + &self.client + } } #[tower_lsp::async_trait] @@ -557,7 +578,7 @@ pub(crate) fn start_lsp( let (shutdown_tx, mut shutdown_rx) = tokio::sync::mpsc::channel::<()>(1); let init = |client: Client| { - let state = GlobalState::new(client, r_home, console_notification_tx); + let state = GlobalState::new(client.clone(), r_home, console_notification_tx); let events_tx = state.events_tx(); // Start main loop and hold onto the handle that keeps it alive @@ -579,6 +600,7 @@ pub(crate) fn start_lsp( Backend { shutdown_tx, events_tx, + client, _main_loop: main_loop, } }; diff --git a/crates/ark/src/r_task.rs b/crates/ark/src/r_task.rs index 0420156a66..c5600334ef 100644 --- a/crates/ark/src/r_task.rs +++ b/crates/ark/src/r_task.rs @@ -24,13 +24,11 @@ use crate::console::Console; use crate::console::ConsoleOutputCapture; use crate::fixtures::r_test_init; -/// Task channels for interrupt-time tasks -static INTERRUPT_TASKS: LazyLock = LazyLock::new(TaskChannels::new); - -/// Task channels for idle-time tasks +/// Task channels for idle-time tasks (top-level only) static IDLE_TASKS: LazyLock = LazyLock::new(TaskChannels::new); -/// Task channels for idle tasks that run at any idle prompt (top-level or browser) +/// Task channels for idle tasks that run at any idle prompt (top-level or browser, but +/// not input) static IDLE_ANY_TASKS: LazyLock = LazyLock::new(TaskChannels::new); // Compared to `futures::BoxFuture`, this doesn't require the future to be Send. @@ -64,19 +62,11 @@ impl TaskChannels { } } -/// Returns receivers for interrupt, idle, and debug-idle tasks. +/// Returns receivers for idle and idle-any tasks. /// Initializes the task channels if they haven't been initialized yet. /// Can only be called once (intended for `Console` during init). -pub(crate) fn take_receivers() -> ( - Receiver, - Receiver, - Receiver, -) { - ( - INTERRUPT_TASKS.take_rx(), - IDLE_TASKS.take_rx(), - IDLE_ANY_TASKS.take_rx(), - ) +pub(crate) fn take_receivers() -> (Receiver, Receiver) { + (IDLE_TASKS.take_rx(), IDLE_ANY_TASKS.take_rx()) } pub enum QueuedRTask { @@ -148,7 +138,7 @@ impl std::task::Wake for RTaskWaker { } impl RTaskStartInfo { - pub(crate) fn new(idle: bool) -> Self { + pub(crate) fn new() -> Self { let thread = std::thread::current(); let thread_id = thread.id(); let thread_name = thread @@ -158,7 +148,7 @@ impl RTaskStartInfo { .to_owned(); let start_time = std::time::Instant::now(); - let span = tracing::trace_span!("R task", thread = thread_name, interrupt = !idle,); + let span = tracing::trace_span!("R task", thread = thread_name); Self { thread_id, @@ -191,6 +181,9 @@ impl RTaskStartInfo { // running, so borrowing is allowed even though we send it to another // thread. See also `Crossbeam::thread::ScopedThreadBuilder` (from which // `r_task()` is adapted) for a similar approach. +// +// `r_task()`s run via `IDLE_ANY_TASKS`, i.e. they run at top level, and at a debugger +// prompt (but notably not the input prompt as a way to reduce risk of reentrancy). pub fn r_task<'env, F, T>(f: F) -> T where @@ -242,9 +235,9 @@ where let task = QueuedRTask::Sync(RTaskSync { fun: closure, status_tx: Some(status_tx), - start_info: RTaskStartInfo::new(false), + start_info: RTaskStartInfo::new(), }); - INTERRUPT_TASKS.tx().send(task).unwrap(); + IDLE_ANY_TASKS.tx().send(task).unwrap(); // Block until we get the signal that the task has started let status = status_rx.recv().unwrap(); @@ -289,23 +282,18 @@ where /// An async task to be run on the R thread. /// -/// Construct via `RTask::interrupt`, `RTask::idle`, or `RTask::idle_any_prompt` -/// when spawning from the R thread. Use the `Send` variants -/// (`RTask::send_interrupt`, etc.) when spawning from other threads. +/// Construct via [RTask::idle] or [RTask::idle_any_prompt] when spawning from the R +/// thread. Use the `Send` variants ([RTask::send_idle], etc.) when spawning from other +/// threads. /// -/// For idle modes, console output is automatically captured during the task's -/// execution via a `ConsoleOutputCapture` passed to the closure. +/// Console output is automatically captured during the task's execution via a +/// `ConsoleOutputCapture` passed to the closure. pub(crate) enum RTask { - /// Run at the next interrupt check. Must be spawned from the R thread. - Interrupt(BoxFuture<'static, ()>), /// Run when R is at a top-level idle prompt. Must be spawned from the R thread. Idle(BoxFuture<'static, ()>), /// Run when R is at any idle prompt (top-level or browser). Must be spawned /// from the R thread. IdleAnyPrompt(BoxFuture<'static, ()>), - /// Like `Interrupt`, but can be spawned from any thread. The constructor - /// enforces `Send` on the closure. - SendInterrupt(BoxFuture<'static, ()>), /// Like `Idle`, but can be spawned from any thread. The constructor /// enforces `Send` on the closure. SendIdle(BoxFuture<'static, ()>), @@ -315,14 +303,6 @@ pub(crate) enum RTask { } impl RTask { - pub(crate) fn interrupt(fun: F) -> Self - where - F: FnOnce() -> Fut + 'static, - Fut: Future + 'static, - { - RTask::Interrupt(Box::pin(fun())) - } - pub(crate) fn idle(fun: F) -> Self where F: FnOnce(ConsoleOutputCapture) -> Fut + 'static, @@ -340,61 +320,62 @@ impl RTask { RTask::IdleAnyPrompt(Self::pin_with_capture(fun)) } - fn pin_with_capture(fun: F) -> BoxFuture<'static, ()> + pub(crate) fn send_idle(fun: F) -> Self where - F: FnOnce(ConsoleOutputCapture) -> Fut + 'static, + F: FnOnce(ConsoleOutputCapture) -> Fut + 'static + Send, Fut: Future + 'static, { - Box::pin(async move { - let capture = if Console::is_initialized() { - Console::get_mut().start_capture() - } else { - // Unit tests run without a Console. The dummy capture is - // inert and doesn't interact with Console state. - debug_assert!(stdext::IS_TESTING); - ConsoleOutputCapture::dummy() - }; - fun(capture).await - }) + RTask::SendIdle(Self::pin_with_capture(fun)) } - pub(crate) fn send_interrupt(fun: F) -> Self + /// For rare performance sensitive cases where you'd like to avoid the cost of + /// poking R options via [Self::pin_with_capture()] and you know you don't need + /// the safety of capturing output because you aren't running R code + pub(crate) fn send_idle_without_capture(fun: F) -> Self where F: FnOnce() -> Fut + 'static + Send, Fut: Future + 'static, { - RTask::SendInterrupt(Box::pin(fun())) + RTask::SendIdle(Box::pin(fun())) } - pub(crate) fn send_idle(fun: F) -> Self + pub(crate) fn send_idle_any_prompt(fun: F) -> Self where F: FnOnce(ConsoleOutputCapture) -> Fut + 'static + Send, Fut: Future + 'static, { - RTask::SendIdle(Self::pin_with_capture(fun)) + RTask::SendIdleAnyPrompt(Self::pin_with_capture(fun)) } - pub(crate) fn send_idle_any_prompt(fun: F) -> Self + fn pin_with_capture(fun: F) -> BoxFuture<'static, ()> where - F: FnOnce(ConsoleOutputCapture) -> Fut + 'static + Send, + F: FnOnce(ConsoleOutputCapture) -> Fut + 'static, Fut: Future + 'static, { - RTask::SendIdleAnyPrompt(Self::pin_with_capture(fun)) + Box::pin(async move { + let capture = if Console::is_initialized() { + Console::get_mut().start_capture() + } else { + // Unit tests run without a Console. The dummy capture is + // inert and doesn't interact with Console state. + debug_assert!(stdext::IS_TESTING); + ConsoleOutputCapture::dummy() + }; + fun(capture).await + }) } } /// Spawn an async task on the R thread. /// -/// For `Send` variants (`RTask::send_interrupt`, etc.) this can be called from +/// For `Send` variants ([RTask::send_idle], etc.) this can be called from /// any thread. Non-`Send` variants must be called from the R thread. pub(crate) fn spawn(task: RTask) { if stdext::IS_TESTING && !Console::is_initialized() { let _lock = harp::fixtures::R_TEST_LOCK.lock(); let fut = match task { - RTask::Interrupt(fut) | RTask::Idle(fut) | RTask::IdleAnyPrompt(fut) | - RTask::SendInterrupt(fut) | RTask::SendIdle(fut) | RTask::SendIdleAnyPrompt(fut) => fut, }; @@ -402,28 +383,22 @@ pub(crate) fn spawn(task: RTask) { return; } - let needs_r_thread = matches!( - task, - RTask::Interrupt(_) | RTask::Idle(_) | RTask::IdleAnyPrompt(_) - ); + let needs_r_thread = matches!(task, RTask::Idle(_) | RTask::IdleAnyPrompt(_)); if needs_r_thread && !Console::on_main_thread() { let thread = std::thread::current(); let name = thread.name().unwrap_or(""); panic!("`spawn()` must be called from the R thread, not thread '{name}'"); } - let (fut, tasks_tx, only_idle) = match task { - RTask::Interrupt(fut) | RTask::SendInterrupt(fut) => (fut, INTERRUPT_TASKS.tx(), false), - RTask::Idle(fut) | RTask::SendIdle(fut) => (fut, IDLE_TASKS.tx(), true), - RTask::IdleAnyPrompt(fut) | RTask::SendIdleAnyPrompt(fut) => { - (fut, IDLE_ANY_TASKS.tx(), true) - }, + let (fut, tasks_tx) = match task { + RTask::Idle(fut) | RTask::SendIdle(fut) => (fut, IDLE_TASKS.tx()), + RTask::IdleAnyPrompt(fut) | RTask::SendIdleAnyPrompt(fut) => (fut, IDLE_ANY_TASKS.tx()), }; let task = QueuedRTask::Async(RTaskAsync { fut, tasks_tx: tasks_tx.clone(), - start_info: RTaskStartInfo::new(only_idle), + start_info: RTaskStartInfo::new(), }); tasks_tx.send(task).unwrap(); diff --git a/crates/ark/src/sys/unix/console.rs b/crates/ark/src/sys/unix/console.rs index d624f32369..533b568a3f 100644 --- a/crates/ark/src/sys/unix/console.rs +++ b/crates/ark/src/sys/unix/console.rs @@ -11,6 +11,7 @@ use std::sync::Condvar; use std::sync::Mutex; use libr::ptr_R_Busy; +use libr::ptr_R_ProcessEvents; use libr::ptr_R_ReadConsole; use libr::ptr_R_ShowMessage; use libr::ptr_R_Suicide; @@ -23,7 +24,6 @@ use libr::R_HomeDir; use libr::R_InputHandlers; use libr::R_Interactive; use libr::R_Outputfile; -use libr::R_PolledEvents; use libr::R_SignalHandlers; use libr::R_checkActivity; use libr::R_runHandlers; @@ -32,7 +32,7 @@ use libr::R_wait_usec; use libr::Rf_initialize_R; use crate::console::r_busy; -use crate::console::r_polled_events; +use crate::console::r_process_events; use crate::console::r_read_console; use crate::console::r_show_message; use crate::console::r_suicide; @@ -77,6 +77,7 @@ pub fn setup_r(args: &Vec) { libr::set(ptr_R_ShowMessage, Some(r_show_message)); libr::set(ptr_R_Busy, Some(r_busy)); libr::set(ptr_R_Suicide, Some(r_suicide)); + libr::set(ptr_R_ProcessEvents, Some(r_process_events)); // Install a CleanUp hook for integration tests that test the shutdown process. // We confirm that shutdown occurs by waiting in the test until `CLEANUP_SIGNAL`'s @@ -97,6 +98,15 @@ pub fn setup_r(args: &Vec) { libr::set(libr::R_CStackLimit, usize::MAX); } + // Set for exactly 1 reason, so that `Rsleep()` on Unix will use it as the + // interval to call `R_CheckUserInterrupt()` (and therefore our + // `R_ProcessEvents()` hook) at while `Sys.sleep()` is running. This allows + // `debug_filter` to flush during a long sleep. Not needed on Windows because + // `R_wait_usec` doesn't exist there, and because `Rsleep()` regularly calls + // `R_ProcessEvents()` directly every 500ms (hardcoded). The test + // `test_adversarial_cat_before_long_sleep` fails without this. + libr::set(R_wait_usec, 10000); + // Set up main loop setup_Rmainloop(); } @@ -104,31 +114,32 @@ pub fn setup_r(args: &Vec) { pub fn run_r() { unsafe { - // Listen for polled events - libr::set(R_wait_usec, 10000); - libr::set(R_PolledEvents, Some(r_polled_events)); - run_Rmainloop(); } } +/// Run handlers if we have data available. This is necessary +/// for things like the HTML help server, which will listen +/// for requests on an open socket() which would then normally +/// be handled in a select() call when reading input from stdin. +/// +/// https://github.com/wch/r-source/blob/4ca6439c1ffc76958592455c44d83f95d5854b2a/src/unix/sys-std.c#L1084-L1086 +/// +/// We run this in a loop just to make sure the R help server can +/// be as responsive as possible when rendering help pages. +/// +/// Note that the later package also adds an input handler to `R_InputHandlers` +/// which runs the later event loop, so it's also important that we are fairly +/// responsive for that as well (posit-dev/positron#7235). +/// +/// Note that `R_runHandlers()` would call `R_PolledEvents()` if we give it a `NULL` +/// `fdset` and we don't want necessarily want this, though in practice it would probably +/// be fine since we don't register anything for `R_PolledEvents()`, making it a no-op by +/// default. +/// https://github.com/wch/r-source/blob/0cd50b1014de382cc27cf72b0e418565f611334a/src/unix/sys-std.c#L408 pub fn run_activity_handlers() { unsafe { - // Run handlers if we have data available. This is necessary - // for things like the HTML help server, which will listen - // for requests on an open socket() which would then normally - // be handled in a select() call when reading input from stdin. - // - // https://github.com/wch/r-source/blob/4ca6439c1ffc76958592455c44d83f95d5854b2a/src/unix/sys-std.c#L1084-L1086 - // - // We run this in a loop just to make sure the R help server can - // be as responsive as possible when rendering help pages. - // - // Note that the later package also adds an input handler to `R_InputHandlers` - // which runs the later event loop, so it's also important that we are fairly - // responsive for that as well (posit-dev/positron#7235). let mut fdset = R_checkActivity(0, 1); - while !fdset.is_null() { R_runHandlers(libr::get(R_InputHandlers), fdset); fdset = R_checkActivity(0, 1); diff --git a/crates/ark/src/sys/windows/console.rs b/crates/ark/src/sys/windows/console.rs index be5ca6d307..42a6065a52 100644 --- a/crates/ark/src/sys/windows/console.rs +++ b/crates/ark/src/sys/windows/console.rs @@ -28,6 +28,7 @@ use regex::bytes::Regex; use super::strings::code_page_to_utf8; use super::strings::get_system_code_page; use crate::console::r_busy; +use crate::console::r_process_events; use crate::console::r_read_console; use crate::console::r_show_message; use crate::console::r_suicide; @@ -114,10 +115,11 @@ pub fn setup_r(args: &Vec) { (*params).Busy = Some(r_busy); (*params).Suicide = Some(r_suicide); - // This is assigned to `ptr_ProcessEvents` (which we don't set on Unix), - // in `R_SetParams()` by `R_SetWin32()` and gets called by `R_ProcessEvents()`. - // It gets called unconditionally, so we have to set it to something, even if a no-op. - (*params).CallBack = Some(r_callback); + // This is assigned to `ptr_ProcessEvents` in `R_SetParams()` by `R_SetWin32()` + // and gets called by `R_ProcessEvents()`. Note that it gets called + // unconditionally, so we have to set it to something, even if a no-op. Keep that + // in mind if we ever get rid of `r_process_events()`! + (*params).CallBack = Some(r_process_events); (*params).rhome = r_home; (*params).home = user_home; @@ -209,11 +211,6 @@ fn get_user_home() -> String { path.to_string() } -#[cfg_attr(not(test), no_mangle)] -extern "C-unwind" fn r_callback() { - // Do nothing! -} - #[cfg_attr(not(test), no_mangle)] extern "C-unwind" fn r_yes_no_cancel(question: *const c_char) -> c_int { // This seems to only be used on Windows during R's default `CleanUp` when diff --git a/crates/ark/src/thread.rs b/crates/ark/src/thread.rs index 4404a86403..dbb707d7d5 100644 --- a/crates/ark/src/thread.rs +++ b/crates/ark/src/thread.rs @@ -86,9 +86,8 @@ impl Drop for RThreadSafe { }; // In tests we're already on the R thread, so drop directly. - // Going through `spawn_interrupt` would call `block_on` which - // panics if we're already inside an executor (e.g. from - // `spawn_idle`'s test path). + // Going through `r_task::spawn()` would call `block_on()` which + // panics if we're already inside an executor. if stdext::IS_TESTING { drop(shelter); return; @@ -96,11 +95,14 @@ impl Drop for RThreadSafe { let _span = tracing::trace_span!("async drop").entered(); - r_task::spawn(RTask::send_interrupt(async move || { - // Run the `drop()` method of the `RShelter`, which in turn - // runs the `drop()` method of the wrapped Rust object, which likely - // uses the R API (i.e. if it is an `RObject`) so it must be called - // on the main R thread. + r_task::spawn(RTask::send_idle_without_capture(async move || { + // Run the `drop()` method of the `RShelter`, which in turn runs the `drop()` + // method of the wrapped Rust object, which likely uses the R API (i.e. if it + // is an `RObject`) so it must be called on the main R thread. By using + // `send_idle_without_capture()`, it is asynchronously dropped at the next + // idle top-level prompt. We use the `send_idle_without_capture()` variant to + // avoid the overhead of `send_idle()`'s `pin_with_capture()`, because this + // runs quite often and we know we don't need to capture output. drop(shelter); })) } diff --git a/crates/ark/tests/dap_breakpoints.rs b/crates/ark/tests/dap_breakpoints.rs index 613fbe47dd..18d63faf55 100644 --- a/crates/ark/tests/dap_breakpoints.rs +++ b/crates/ark/tests/dap_breakpoints.rs @@ -5,8 +5,10 @@ // // +use amalthea::fixtures::dummy_frontend::ExecuteRequestOptions; use ark_test::DummyArkFrontend; use ark_test::SourceFile; +use tower_lsp::lsp_types; #[test] fn test_dap_set_breakpoints_unverified() { @@ -238,3 +240,88 @@ fn test_dap_breakpoints_unsaved_file_unverified() { .unwrap() .contains("Can't read file")); } + +/// When the LSP signals a document change while R is paused at a `browser()` prompt, DAP +/// must still invalidate that file's breakpoints. The `process_console_notifications()` +/// task is routed through `IDLE_ANY_TASKS` so it drains at browser prompts. +#[test] +fn test_dap_an_lsp_did_change_notification_at_browser_invalidates_breakpoints() { + let frontend = DummyArkFrontend::lock(); + let mut dap = frontend.start_dap(); + let mut lsp = frontend.start_lsp(); + + let file = SourceFile::new( + " +foo <- function() { + browser() + 1 +} +foo() +", + ); + + // Set a breakpoint on the line after `browser()`, then source the file. The + // breakpoint becomes verified when the function definition is evaluated. The call to + // `foo()` then pauses R at `browser()`. + let breakpoints = dap.set_breakpoints(&file.path, &[4]); + assert_eq!(breakpoints.len(), 1); + let bp_id = breakpoints[0].id; + + frontend.send_execute_request( + &format!("source('{}')", file.path), + ExecuteRequestOptions::default(), + ); + frontend.recv_iopub_busy(); + frontend.recv_iopub_execute_input(); + + let bp = dap.recv_breakpoint_verified(); + assert_eq!(bp.id, bp_id); + + frontend.recv_iopub_start_debug(); + frontend.recv_iopub_idle(); + frontend.recv_shell_execute_reply(); + dap.recv_stopped(); + + // Open the file on the LSP using its real URL so the LSP-side `UrlId` + // canonicalizes to the same one the DAP indexed the breakpoint under. + let uri = lsp_types::Url::from_file_path(&file.path).unwrap(); + lsp.send_notification( + "textDocument/didOpen", + serde_json::to_value(lsp_types::DidOpenTextDocumentParams { + text_document: lsp_types::TextDocumentItem { + uri: uri.clone(), + language_id: String::from("r"), + version: 0, + text: String::from("x <- 1\n"), + }, + }) + .unwrap(), + ); + + // Send a full-document change. The LSP `did_change` handler forwards a + // `ConsoleNotification::DidChangeDocument` to the console, which the DAP + // turns into a `BreakpointState{verified:false}` event. + lsp.send_notification( + "textDocument/didChange", + serde_json::to_value(lsp_types::DidChangeTextDocumentParams { + text_document: lsp_types::VersionedTextDocumentIdentifier { + uri: uri.clone(), + version: 1, + }, + content_changes: vec![lsp_types::TextDocumentContentChangeEvent { + range: None, + range_length: None, + text: String::from("x <- 2\n"), + }], + }) + .unwrap(), + ); + + // The event must arrive while we are still paused in the browser + let bp = dap.recv_breakpoint_event(); + assert_eq!(bp.id, bp_id); + assert!(!bp.verified); + + frontend.debug_send_quit(); + dap.recv_continued(); +} diff --git a/crates/ark/tests/kernel-debugger.rs b/crates/ark/tests/kernel-debugger.rs index 45892b4501..209aaab55a 100644 --- a/crates/ark/tests/kernel-debugger.rs +++ b/crates/ark/tests/kernel-debugger.rs @@ -640,3 +640,32 @@ evalq(base::browser(), env)"; frontend.recv_iopub_idle(); assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count); } + +/// `r_task()` calls from non-R threads must run while R is paused at a `browser()` +/// prompt. This is whole point of `IDLE_ANY_TASKS`! Without it, requests from the +/// LSP/DAP/Variables threads would silently hang for the duration of every debug session. +#[test] +fn test_browser_with_r_task() { + let frontend = DummyArkFrontend::lock(); + + frontend.send_execute_request("browser()", ExecuteRequestOptions::default()); + frontend.recv_iopub_busy(); + frontend.recv_iopub_execute_input(); + frontend.recv_iopub_idle(); + frontend.recv_shell_execute_reply(); + + // R is now paused at the browser prompt. Issue an `r_task()` from a worker thread. + // `IDLE_ANY_TASKS` should drain at browser prompts, so this completes promptly. + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let value = ark::r_task::r_task(|| 42); + let _ = tx.send(value); + }); + + let value = rx + .recv_timeout(std::time::Duration::from_secs(5)) + .expect("`r_task()` should run while R is paused at a `browser()` prompt"); + assert_eq!(value, 42); + + frontend.execute_request_invisibly("Q"); +} diff --git a/crates/ark/tests/kernel-stdin.rs b/crates/ark/tests/kernel-stdin.rs index 47a2619a73..83dbb2537b 100644 --- a/crates/ark/tests/kernel-stdin.rs +++ b/crates/ark/tests/kernel-stdin.rs @@ -191,3 +191,58 @@ fn test_stdin_readline_during_autoprint() { assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count); } + +/// `r_task()` calls from non-R threads are intentionally NOT processed while R +/// is at an input-request prompt (e.g. `readline()` / `menu()`). The task +/// channels drained inside `run_event_loop()` are gated to top-level and +/// browser prompts only. A pending `r_task()` waits until R returns to one of +/// those prompts before running. +/// +/// The goal of not running tasks at the input request prompt is to avoid too +/// much reentrancy risk. +#[test] +fn test_r_task_does_not_run_at_input_request_prompt() { + let frontend = DummyArkFrontend::lock(); + + let options = ExecuteRequestOptions { + allow_stdin: true, + ..Default::default() + }; + + let code = "readline('prompt>')"; + frontend.send_execute_request(code, options); + frontend.recv_iopub_busy(); + + let input = frontend.recv_iopub_execute_input(); + assert_eq!(input.code, code); + + let prompt = frontend.recv_stdin_input_request(); + assert_eq!(prompt, String::from("prompt>")); + + // R is now blocked at the input-request prompt. Issue an `r_task()` from + // a worker thread. The closure is trivial, so if tasks were drained here + // it would complete near-instantly. + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let value = ark::r_task::r_task(|| 42); + let _ = tx.send(value); + }); + + // The task should NOT complete while R is at the input-request prompt. + assert_eq!( + rx.recv_timeout(std::time::Duration::from_millis(500)), + Err(std::sync::mpsc::RecvTimeoutError::Timeout) + ); + + // Respond to the input-request prompt so R returns to the top-level prompt + frontend.send_stdin_input_reply(String::from("hi")); + frontend.recv_iopub_execute_result(); + frontend.recv_iopub_idle(); + frontend.recv_shell_execute_reply(); + + // Once R is back at the top-level prompt, the queued task should run. + let value = rx + .recv_timeout(std::time::Duration::from_secs(5)) + .expect("`r_task()` should run once R returns to a top-level prompt"); + assert_eq!(value, 42); +} diff --git a/crates/ark/tests/stream_filter.rs b/crates/ark/tests/stream_filter.rs index 27be84f13c..3413eaacbf 100644 --- a/crates/ark/tests/stream_filter.rs +++ b/crates/ark/tests/stream_filter.rs @@ -576,6 +576,47 @@ fn test_adversarial_cat_interleaved_with_normal() { frontend.recv_shell_execute_reply(); } +/// For `Sys.sleep()` on Unix in particular, we must set `R_wait_usec` so that `Rsleep()` +/// calls `R_CheckUserInterrupt()` (and therefore our `R_ProcessEvents()` hook) regularly. +/// Otherwise, the default behavior with an unset `R_wait_usec` is to just block until the +/// sleep is finished, and then check for interrupts once on the way out. +/// https://github.com/wch/r-source/blob/62ee67d861d80e8985227a51dc032d8d9a8115f7/src/unix/sys-std.c#L1488 +/// +/// If R blocks without checking for interrupts, our adversarial user output of `debug at +/// foo` won't get flushed because we won't be able to check that the `ConsoleFilter` +/// timeout is hit (500ms in test mode). +/// +/// On Windows, `Rsleep()` calls `R_ProcessEvents()` on a hardcoded 500ms interval, so +/// this test should also pass there with no issues. +/// https://github.com/wch/r-source/blob/62ee67d861d80e8985227a51dc032d8d9a8115f7/src/gnuwin32/extra.c#L363-L366 +#[test] +fn test_adversarial_cat_before_long_sleep() { + let frontend = DummyArkFrontend::lock(); + + let code = r#"{ + cat("debug at foo") + Sys.sleep(5) + }"#; + + let start = std::time::Instant::now(); + frontend.send_execute_request(code, ExecuteRequestOptions::default()); + frontend.recv_iopub_busy(); + frontend.recv_iopub_execute_input(); + + // Buffered text should arrive ~500 ms (`ConsoleFilter` timeout in test mode) + // after `cat()` runs, not after the 5s sleep completes. + frontend.assert_stream_stdout_contains("debug at foo"); + let elapsed = start.elapsed(); + + assert!( + elapsed < std::time::Duration::from_secs(5), + "Expected 'debug at foo' to flush during Sys.sleep, but it took {elapsed:?}" + ); + + frontend.recv_iopub_idle(); + frontend.recv_shell_execute_reply(); +} + /// When stderr arrives while stdout is buffered in the filter (e.g. partial /// prefix match), the buffered stdout is flushed before the stderr so that /// ordering is preserved. diff --git a/crates/harp/src/exec.rs b/crates/harp/src/exec.rs index 5e1b0b7419..223fcc89d6 100644 --- a/crates/harp/src/exec.rs +++ b/crates/harp/src/exec.rs @@ -509,6 +509,8 @@ where F: 'env, T: 'env, { + // TODO: These days this just means `RLocalInterruptsSuspended`. + // Should we simplify the name from `r_sandbox()` to `r_interrupts_suspended()`? let _scope = crate::raii::RLocalSandbox::new(); try_catch(f) } diff --git a/crates/harp/src/lib.rs b/crates/harp/src/lib.rs index c3859eb489..511d0eb38c 100644 --- a/crates/harp/src/lib.rs +++ b/crates/harp/src/lib.rs @@ -29,7 +29,6 @@ pub mod object; pub mod options; pub mod parse; pub mod parser; -pub mod polled_events; pub mod protect; pub mod r; pub mod raii; diff --git a/crates/harp/src/polled_events.rs b/crates/harp/src/polled_events.rs deleted file mode 100644 index a009af3d25..0000000000 --- a/crates/harp/src/polled_events.rs +++ /dev/null @@ -1,8 +0,0 @@ -// -// polled_events.rs -// -// Copyright (C) 2023 Posit Software, PBC. All rights reserved. -// -// - -pub use crate::sys::polled_events::RLocalPolledEventsSuspended; diff --git a/crates/harp/src/raii.rs b/crates/harp/src/raii.rs index e917ea9219..455441809a 100644 --- a/crates/harp/src/raii.rs +++ b/crates/harp/src/raii.rs @@ -34,7 +34,6 @@ pub struct RLocalInteractive { pub struct RLocalSandbox { _interrupts_scope: RLocalInterruptsSuspended, - _polled_events_scope: crate::sys::polled_events::RLocalPolledEventsSuspended, } pub struct RLocalOptionBoolean { @@ -132,7 +131,6 @@ impl RLocalSandbox { pub fn new() -> Self { Self { _interrupts_scope: RLocalInterruptsSuspended::new(true), - _polled_events_scope: crate::sys::polled_events::RLocalPolledEventsSuspended::new(true), } } } diff --git a/crates/harp/src/sys/unix.rs b/crates/harp/src/sys/unix.rs index e4ebb3fd00..f251ebddc5 100644 --- a/crates/harp/src/sys/unix.rs +++ b/crates/harp/src/sys/unix.rs @@ -8,4 +8,3 @@ pub mod command; pub mod library; pub mod line_ending; -pub mod polled_events; diff --git a/crates/harp/src/sys/unix/polled_events.rs b/crates/harp/src/sys/unix/polled_events.rs deleted file mode 100644 index 8f0f538aa6..0000000000 --- a/crates/harp/src/sys/unix/polled_events.rs +++ /dev/null @@ -1,26 +0,0 @@ -// -// polled_events.rs -// -// Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved. -// -// - -pub struct RLocalPolledEventsSuspended { - _raii: crate::raii::RLocal>, -} - -#[no_mangle] -extern "C-unwind" fn r_polled_events_disabled() {} - -impl RLocalPolledEventsSuspended { - pub fn new(value: bool) -> Self { - let new_value = if value { - Some(r_polled_events_disabled as unsafe extern "C-unwind" fn()) - } else { - None - }; - Self { - _raii: crate::raii::RLocal::new(new_value, unsafe { libr::R_PolledEvents }), - } - } -} diff --git a/crates/harp/src/sys/windows.rs b/crates/harp/src/sys/windows.rs index ec9bec5ac3..e6e6e0f1f6 100644 --- a/crates/harp/src/sys/windows.rs +++ b/crates/harp/src/sys/windows.rs @@ -9,4 +9,3 @@ pub mod command; pub mod library; pub mod line_ending; mod locale; -pub mod polled_events; diff --git a/crates/harp/src/sys/windows/polled_events.rs b/crates/harp/src/sys/windows/polled_events.rs deleted file mode 100644 index 6d06580fd9..0000000000 --- a/crates/harp/src/sys/windows/polled_events.rs +++ /dev/null @@ -1,15 +0,0 @@ -// -// polled_events.rs -// -// Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved. -// -// - -// Polled events aren't used on Windows, so this is a no-op -pub struct RLocalPolledEventsSuspended {} - -impl RLocalPolledEventsSuspended { - pub fn new(_value: bool) -> Self { - Self {} - } -} diff --git a/crates/libr/src/r.rs b/crates/libr/src/r.rs index 0dd1a18367..206274bbd6 100644 --- a/crates/libr/src/r.rs +++ b/crates/libr/src/r.rs @@ -99,8 +99,6 @@ functions::generate! { pub fn R_PreserveObject(arg1: SEXP); - pub fn R_RunPendingFinalizers(); - pub fn R_ToplevelExec( fun: Option, data: *mut std::ffi::c_void @@ -744,9 +742,6 @@ mutable_globals::generate! { #[cfg(target_family = "unix")] pub static mut R_wait_usec: i32; - #[cfg(target_family = "unix")] - pub static mut R_PolledEvents: Option; - #[cfg(target_family = "unix")] pub static mut ptr_R_WriteConsole: Option< unsafe extern "C-unwind" fn(arg1: *const std::ffi::c_char, arg2: std::ffi::c_int), @@ -783,6 +778,9 @@ mutable_globals::generate! { #[cfg(target_family = "unix")] pub static mut ptr_R_CleanUp: Option; + #[cfg(target_family = "unix")] + pub static mut ptr_R_ProcessEvents: Option; + // ----------------------------------------------------------------------------------- // Windows