Skip to content

Commit 528a55a

Browse files
committed
Send interrupt before shutting down
1 parent dc854fe commit 528a55a

File tree

2 files changed

+50
-0
lines changed

2 files changed

+50
-0
lines changed

crates/ark/src/control.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ impl ControlHandler for Control {
3636
) -> Result<ShutdownReply, Exception> {
3737
log::info!("Received shutdown request: {msg:?}");
3838

39+
// Interrupt any ongoing computation. We shut down from ReadConsole when
40+
// R has become idle again. Note that Positron will have interrupted us
41+
// beforehand, but another frontend might not have, and it's good to
42+
// have this as a defensive measure in any case.
43+
crate::sys::control::handle_interrupt_request();
44+
3945
// According to the Jupyter protocol we should block here until the
4046
// shutdown is complete. However AFAICS ipykernel doesn't wait
4147
// until complete shutdown before replying and instead just signals

crates/ark/tests/kernel.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ use amalthea::wire::jupyter_message::Message;
33
use amalthea::wire::jupyter_message::Status;
44
use amalthea::wire::kernel_info_request::KernelInfoRequest;
55
use ark::fixtures::DummyArkFrontend;
6+
use nix::sys::signal::signal;
7+
use nix::sys::signal::SigHandler;
8+
use nix::sys::signal::Signal;
69
use stdext::assert_match;
710

811
#[test]
@@ -1145,10 +1148,21 @@ fn test_env_vars() {
11451148
assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count);
11461149
}
11471150

1151+
/// Install a SIGINT handler for shutdown tests. This overrides the test runner
1152+
/// handler so it doesn't cancel our test.
1153+
fn install_sigint_handler() {
1154+
extern "C" fn sigint_handler(_: libc::c_int) {}
1155+
#[cfg(unix)]
1156+
unsafe {
1157+
signal(Signal::SIGINT, SigHandler::Handler(sigint_handler)).unwrap();
1158+
}
1159+
}
1160+
11481161
// Note that because of these shutdown tests you _have_ to use `cargo nextest`
11491162
// instead of `cargo test`, so that each test has its own process and R thread.
11501163
#[test]
11511164
fn test_shutdown_request() {
1165+
install_sigint_handler();
11521166
let frontend = DummyArkFrontend::lock();
11531167

11541168
frontend.send_shutdown_request(false);
@@ -1165,6 +1179,7 @@ fn test_shutdown_request() {
11651179

11661180
#[test]
11671181
fn test_shutdown_request_with_restart() {
1182+
install_sigint_handler();
11681183
let frontend = DummyArkFrontend::lock();
11691184

11701185
frontend.send_shutdown_request(true);
@@ -1183,6 +1198,7 @@ fn test_shutdown_request_with_restart() {
11831198
// https://github.com/posit-dev/positron/issues/6553
11841199
#[test]
11851200
fn test_shutdown_request_browser() {
1201+
install_sigint_handler();
11861202
let frontend = DummyArkFrontend::lock();
11871203

11881204
let code = "browser()";
@@ -1211,3 +1227,31 @@ fn test_shutdown_request_browser() {
12111227

12121228
DummyArkFrontend::wait_for_cleanup();
12131229
}
1230+
1231+
#[test]
1232+
fn test_shutdown_request_while_busy() {
1233+
install_sigint_handler();
1234+
let frontend = DummyArkFrontend::lock();
1235+
1236+
let code = "Sys.sleep(10)";
1237+
frontend.send_execute_request(code, ExecuteRequestOptions::default());
1238+
frontend.recv_iopub_busy();
1239+
1240+
let input = frontend.recv_iopub_execute_input();
1241+
assert_eq!(input.code, code);
1242+
1243+
frontend.send_shutdown_request(false);
1244+
frontend.recv_iopub_busy();
1245+
1246+
let reply = frontend.recv_control_shutdown_reply();
1247+
assert_eq!(reply.status, Status::Ok);
1248+
assert_eq!(reply.restart, false);
1249+
1250+
frontend.recv_iopub_stream_stderr("\n");
1251+
frontend.recv_iopub_idle();
1252+
1253+
assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count);
1254+
frontend.recv_iopub_idle();
1255+
1256+
DummyArkFrontend::wait_for_cleanup();
1257+
}

0 commit comments

Comments
 (0)