Skip to content

Commit 749f940

Browse files
committed
Split RMain::setup() from RMain::start()
This way the former can be run in a background thread whereas the latter can still run in the main thread and propagate panics
1 parent 89ffca7 commit 749f940

File tree

4 files changed

+40
-22
lines changed

4 files changed

+40
-22
lines changed

crates/ark/src/fixtures/dummy_frontend.rs

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,24 +42,25 @@ impl DummyArkFrontend {
4242
let frontend = DummyFrontend::new();
4343
let connection_file = frontend.get_connection_file();
4444

45+
// Start the kernel in this thread so that panics are propagated
46+
crate::start::start_kernel(
47+
connection_file,
48+
vec![
49+
String::from("--interactive"),
50+
String::from("--vanilla"),
51+
String::from("--no-save"),
52+
String::from("--no-restore"),
53+
],
54+
None,
55+
SessionMode::Console,
56+
false,
57+
);
58+
59+
// Start the REPL in a background thread, does not return and is never joined
4560
stdext::spawn!("dummy_kernel", || {
46-
crate::start::start_kernel(
47-
connection_file,
48-
vec![
49-
String::from("--interactive"),
50-
String::from("--vanilla"),
51-
String::from("--no-save"),
52-
String::from("--no-restore"),
53-
],
54-
None,
55-
SessionMode::Console,
56-
false,
57-
);
61+
RMain::start();
5862
});
5963

60-
// Wait for startup to complete
61-
RMain::wait_r_initialized();
62-
6364
frontend.complete_initialization();
6465
frontend
6566
}

crates/ark/src/interface.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,9 +282,14 @@ pub enum ConsoleResult {
282282
}
283283

284284
impl RMain {
285-
/// Starts the main R thread and initializes the `R_MAIN` singleton.
286-
/// Doesn't return. Must be called only once.
287-
pub fn start(
285+
/// Sets up the main R thread and initializes the `R_MAIN` singleton. Must
286+
/// be called only once. This is doing as much setup as possible before
287+
/// starting the R REPL. Since the REPL does not return, it might be
288+
/// launched in a background thread (which we do in integration tests). The
289+
/// setup can still be done in your main thread so that panics may propagate
290+
/// as expected. Call `RMain::start_repl()` after this to actually start the
291+
/// R REPL.
292+
pub fn setup(
288293
r_args: Vec<String>,
289294
startup_file: Option<String>,
290295
kernel_mutex: Arc<Mutex<Kernel>>,
@@ -423,8 +428,13 @@ impl RMain {
423428
if !ignore_user_r_profile {
424429
startup::source_user_r_profile();
425430
}
431+
}
426432

427-
// Does not return!
433+
/// Start the REPL. Does not return!
434+
pub fn start() {
435+
// Update the main thread ID in case the REPL is started in a background
436+
// thread (e.g. in integration tests)
437+
unsafe { R_MAIN_THREAD_ID = Some(std::thread::current().id()) };
428438
crate::sys::interface::run_r();
429439
}
430440

crates/ark/src/main.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use std::env;
1212

1313
use amalthea::connection_file::ConnectionFile;
1414
use amalthea::kernel_spec::KernelSpec;
15+
use ark::interface::RMain;
1516
use ark::interface::SessionMode;
1617
use ark::logger;
1718
use ark::signals::initialize_signal_block;
@@ -315,13 +316,18 @@ fn parse_file(
315316
Ok(connection) => {
316317
log::info!("Loaded connection information from frontend in {connection_file}");
317318
log::info!("Connection data: {:?}", connection);
319+
320+
// Set up R and start the Jupyter kernel
318321
start_kernel(
319322
connection,
320323
r_args,
321324
startup_file,
322325
session_mode,
323326
capture_streams,
324327
);
328+
329+
// Start the REPL, does not return
330+
RMain::start();
325331
},
326332
Err(error) => {
327333
log::error!("Couldn't read connection file {connection_file}: {error:?}");

crates/ark/src/start.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ use crate::request::KernelRequest;
2222
use crate::request::RRequest;
2323
use crate::shell::Shell;
2424

25-
/// Exported for unit tests. Does not return.
25+
/// Exported for unit tests.
26+
/// Call `RMain::start_repl()` after this.
2627
pub fn start_kernel(
2728
connection_file: ConnectionFile,
2829
r_args: Vec<String>,
@@ -112,8 +113,8 @@ pub fn start_kernel(
112113
panic!("Couldn't connect to frontend: {err:?}");
113114
}
114115

115-
// Start the R REPL (does not return for the duration of the session)
116-
crate::interface::RMain::start(
116+
// Setup the channels and R
117+
crate::interface::RMain::setup(
117118
r_args,
118119
startup_file,
119120
kernel_clone,

0 commit comments

Comments
 (0)