Skip to content

Commit

Permalink
Remove a few more easy static muts (#665)
Browse files Browse the repository at this point in the history
* Move `R_BANNER` into `RMain`

* Move `ERROR_BUF` to `RMain`

This works only because of the `UnsafeCell` around `RMain`

* Remove unused `r_version()`

Seems easier than resolving the `R_VERSION` issues, we can always add it back if we need it, but we don't really need it with our current libr setup.
  • Loading branch information
DavisVaughan authored Jan 23, 2025
1 parent 70bb464 commit 281b8ca
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 76 deletions.
31 changes: 16 additions & 15 deletions crates/ark/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,6 @@ thread_local! {
pub static R_MAIN: RefCell<UnsafeCell<RMain>> = panic!("Must access `R_MAIN` from the R thread");
}

/// Banner output accumulated during startup
static mut R_BANNER: String = String::new();

pub struct RMain {
kernel_init_tx: Bus<KernelInfo>,

Expand Down Expand Up @@ -229,6 +226,13 @@ pub struct RMain {
pub positron_ns: Option<RObject>,

pending_lines: Vec<String>,

/// Banner output accumulated during startup
r_banner: String,

/// Raw error buffer provided to `Rf_error()` when throwing `r_read_console()` errors.
/// Stored in `RMain` to avoid memory leakage when `Rf_error()` jumps.
r_error_buffer: Option<CString>,
}

/// Represents the currently active execution request from the frontend. It
Expand Down Expand Up @@ -513,7 +517,7 @@ impl RMain {

let kernel_info = KernelInfo {
version: version.clone(),
banner: R_BANNER.clone(),
banner: self.r_banner.clone(),
input_prompt: Some(input_prompt),
continuation_prompt: Some(continuation_prompt),
};
Expand Down Expand Up @@ -563,6 +567,8 @@ impl RMain {
session_mode,
positron_ns: None,
pending_lines: Vec::new(),
r_banner: String::new(),
r_error_buffer: None,
}
}

Expand Down Expand Up @@ -1590,14 +1596,14 @@ impl RMain {
Err(err) => panic!("Failed to read from R buffer: {err:?}"),
};

let r_main = RMain::get_mut();

if !RMain::is_initialized() {
// During init, consider all output to be part of the startup banner
unsafe { R_BANNER.push_str(&content) };
r_main.r_banner.push_str(&content);
return;
}

let r_main = RMain::get_mut();

// To capture the current `debug: <call>` output, for use in the debugger's
// match based fallback
r_main.dap.handle_stdout(&content);
Expand Down Expand Up @@ -1950,17 +1956,12 @@ pub extern "C" fn r_read_console(
return 0;
},
ConsoleResult::Error(err) => {
// Save error message to a global buffer to avoid leaking memory
// Save error message to `RMain`'s buffer to avoid leaking memory
// when `Rf_error()` jumps. Some gymnastics are required to deal
// with the possibility of `CString` conversion failure since the
// error message comes from the frontend and might be corrupted.
static mut ERROR_BUF: Option<CString> = None;

unsafe {
ERROR_BUF = Some(new_cstring(format!("\n{err}")));
}

unsafe { Rf_error(ERROR_BUF.as_ref().unwrap().as_ptr()) };
main.r_error_buffer = Some(new_cstring(format!("\n{err}")));
unsafe { Rf_error(main.r_error_buffer.as_ref().unwrap().as_ptr()) };
},
};
}
Expand Down
1 change: 0 additions & 1 deletion crates/harp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ pub mod parse;
pub mod parser;
pub mod polled_events;
pub mod protect;
pub mod r_version;
pub mod raii;
pub mod routines;
pub mod session;
Expand Down
60 changes: 0 additions & 60 deletions crates/harp/src/r_version.rs

This file was deleted.

0 comments on commit 281b8ca

Please sign in to comment.