From 7ba49658f7615cf62891b17155381ac65a6a5c78 Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Wed, 27 Apr 2022 10:44:14 +0200 Subject: [PATCH] fix(ipc): empty ipc msg crash (#1351) * fix(ipc): recover from corrupted channel state * style(fmt): rustfmt --- src/sessions.rs | 9 +- zellij-client/src/lib.rs | 25 ++- zellij-client/src/os_input_output.rs | 4 +- zellij-client/src/unit/input_handler_tests.rs | 2 +- zellij-server/src/route.rs | 154 ++++++++++-------- zellij-utils/src/ipc.rs | 7 +- 6 files changed, 116 insertions(+), 85 deletions(-) diff --git a/src/sessions.rs b/src/sessions.rs index 20dfcc568..2d65925b0 100644 --- a/src/sessions.rs +++ b/src/sessions.rs @@ -58,10 +58,13 @@ fn assert_socket(name: &str) -> bool { Ok(stream) => { let mut sender = IpcSenderWithContext::new(stream); sender.send(ClientToServerMsg::ConnStatus); - let mut receiver: IpcReceiverWithContext = sender.get_receiver(); - let (instruction, _) = receiver.recv(); - matches!(instruction, ServerToClientMsg::Connected) + match receiver.recv() { + Some((instruction, _)) => { + matches!(instruction, ServerToClientMsg::Connected) + } + None => false, + } } Err(e) if e.kind() == io::ErrorKind::ConnectionRefused => { drop(fs::remove_file(path)); diff --git a/zellij-client/src/lib.rs b/zellij-client/src/lib.rs index 556f18120..1f05a9eb2 100644 --- a/zellij-client/src/lib.rs +++ b/zellij-client/src/lib.rs @@ -270,14 +270,23 @@ pub fn start_client( let os_input = os_input.clone(); let mut should_break = false; move || loop { - let (instruction, err_ctx) = os_input.recv_from_server(); - err_ctx.update_thread_ctx(); - if let ServerToClientMsg::Exit(_) = instruction { - should_break = true; - } - send_client_instructions.send(instruction.into()).unwrap(); - if should_break { - break; + match os_input.recv_from_server() { + Some((instruction, err_ctx)) => { + err_ctx.update_thread_ctx(); + if let ServerToClientMsg::Exit(_) = instruction { + should_break = true; + } + send_client_instructions.send(instruction.into()).unwrap(); + if should_break { + break; + } + } + None => { + send_client_instructions + .send(ClientInstruction::UnblockInputThread) + .unwrap(); + log::error!("Received empty message from server"); + } } } }) diff --git a/zellij-client/src/os_input_output.rs b/zellij-client/src/os_input_output.rs index 136e89eb3..b93b6121d 100644 --- a/zellij-client/src/os_input_output.rs +++ b/zellij-client/src/os_input_output.rs @@ -93,7 +93,7 @@ pub trait ClientOsApi: Send + Sync { fn send_to_server(&self, msg: ClientToServerMsg); /// Receives a message on client-side IPC channel // This should be called from the client-side router thread only. - fn recv_from_server(&self) -> (ServerToClientMsg, ErrorContext); + fn recv_from_server(&self) -> Option<(ServerToClientMsg, ErrorContext)>; fn handle_signals(&self, sigwinch_cb: Box, quit_cb: Box); /// Establish a connection with the server socket. fn connect_to_server(&self, path: &Path); @@ -144,7 +144,7 @@ impl ClientOsApi for ClientOsInputOutput { .unwrap() .send(msg); } - fn recv_from_server(&self) -> (ServerToClientMsg, ErrorContext) { + fn recv_from_server(&self) -> Option<(ServerToClientMsg, ErrorContext)> { self.receive_instructions_from_server .lock() .unwrap() diff --git a/zellij-client/src/unit/input_handler_tests.rs b/zellij-client/src/unit/input_handler_tests.rs index 1c5180c81..a8b99713c 100644 --- a/zellij-client/src/unit/input_handler_tests.rs +++ b/zellij-client/src/unit/input_handler_tests.rs @@ -151,7 +151,7 @@ impl ClientOsApi for FakeClientOsApi { command_is_executing.unblock_input_thread(); } } - fn recv_from_server(&self) -> (ServerToClientMsg, ErrorContext) { + fn recv_from_server(&self) -> Option<(ServerToClientMsg, ErrorContext)> { unimplemented!() } fn handle_signals(&self, _sigwinch_cb: Box, _quit_cb: Box) { diff --git a/zellij-server/src/route.rs b/zellij-server/src/route.rs index dfc9ab294..af31523a4 100644 --- a/zellij-server/src/route.rs +++ b/zellij-server/src/route.rs @@ -397,81 +397,97 @@ pub(crate) fn route_thread_main( client_id: ClientId, ) { loop { - let (instruction, err_ctx) = receiver.recv(); - err_ctx.update_thread_ctx(); - let rlocked_sessions = session_data.read().unwrap(); + match receiver.recv() { + Some((instruction, err_ctx)) => { + err_ctx.update_thread_ctx(); + let rlocked_sessions = session_data.read().unwrap(); - match instruction { - ClientToServerMsg::Action(action) => { - if let Some(rlocked_sessions) = rlocked_sessions.as_ref() { - if let Action::SwitchToMode(input_mode) = action { - os_input - .send_to_client(client_id, ServerToClientMsg::SwitchToMode(input_mode)); + match instruction { + ClientToServerMsg::Action(action) => { + if let Some(rlocked_sessions) = rlocked_sessions.as_ref() { + if let Action::SwitchToMode(input_mode) = action { + os_input.send_to_client( + client_id, + ServerToClientMsg::SwitchToMode(input_mode), + ); + } + if route_action( + action, + rlocked_sessions, + &*os_input, + &to_server, + client_id, + ) { + break; + } + } } - if route_action(action, rlocked_sessions, &*os_input, &to_server, client_id) { + ClientToServerMsg::TerminalResize(new_size) => { + session_state + .write() + .unwrap() + .set_client_size(client_id, new_size); + let min_size = session_state + .read() + .unwrap() + .min_client_terminal_size() + .unwrap(); + rlocked_sessions + .as_ref() + .unwrap() + .senders + .send_to_screen(ScreenInstruction::TerminalResize(min_size)) + .unwrap(); + } + ClientToServerMsg::TerminalPixelDimensions(pixel_dimensions) => { + rlocked_sessions + .as_ref() + .unwrap() + .senders + .send_to_screen(ScreenInstruction::TerminalPixelDimensions( + pixel_dimensions, + )) + .unwrap(); + } + ClientToServerMsg::NewClient( + client_attributes, + cli_args, + opts, + layout, + plugin_config, + ) => { + let new_client_instruction = ServerInstruction::NewClient( + client_attributes, + cli_args, + opts, + layout, + client_id, + plugin_config, + ); + to_server.send(new_client_instruction).unwrap(); + } + ClientToServerMsg::AttachClient(client_attributes, opts) => { + let attach_client_instruction = + ServerInstruction::AttachClient(client_attributes, opts, client_id); + to_server.send(attach_client_instruction).unwrap(); + } + ClientToServerMsg::ClientExited => { + // we don't unwrap this because we don't really care if there's an error here (eg. + // if the main server thread exited before this router thread did) + let _ = to_server.send(ServerInstruction::RemoveClient(client_id)); + break; + } + ClientToServerMsg::KillSession => { + to_server.send(ServerInstruction::KillSession).unwrap(); + } + ClientToServerMsg::ConnStatus => { + let _ = to_server.send(ServerInstruction::ConnStatus(client_id)); break; } } } - ClientToServerMsg::TerminalResize(new_size) => { - session_state - .write() - .unwrap() - .set_client_size(client_id, new_size); - let min_size = session_state - .read() - .unwrap() - .min_client_terminal_size() - .unwrap(); - rlocked_sessions - .as_ref() - .unwrap() - .senders - .send_to_screen(ScreenInstruction::TerminalResize(min_size)) - .unwrap(); - } - ClientToServerMsg::TerminalPixelDimensions(pixel_dimensions) => { - rlocked_sessions - .as_ref() - .unwrap() - .senders - .send_to_screen(ScreenInstruction::TerminalPixelDimensions(pixel_dimensions)) - .unwrap(); - } - ClientToServerMsg::NewClient( - client_attributes, - cli_args, - opts, - layout, - plugin_config, - ) => { - let new_client_instruction = ServerInstruction::NewClient( - client_attributes, - cli_args, - opts, - layout, - client_id, - plugin_config, - ); - to_server.send(new_client_instruction).unwrap(); - } - ClientToServerMsg::AttachClient(client_attributes, opts) => { - let attach_client_instruction = - ServerInstruction::AttachClient(client_attributes, opts, client_id); - to_server.send(attach_client_instruction).unwrap(); - } - ClientToServerMsg::ClientExited => { - // we don't unwrap this because we don't really care if there's an error here (eg. - // if the main server thread exited before this router thread did) - let _ = to_server.send(ServerInstruction::RemoveClient(client_id)); - break; - } - ClientToServerMsg::KillSession => { - to_server.send(ServerInstruction::KillSession).unwrap(); - } - ClientToServerMsg::ConnStatus => { - let _ = to_server.send(ServerInstruction::ConnStatus(client_id)); - break; + None => { + log::error!("Received empty message from client"); } } } diff --git a/zellij-utils/src/ipc.rs b/zellij-utils/src/ipc.rs index 3d95cd838..8a3dafd36 100644 --- a/zellij-utils/src/ipc.rs +++ b/zellij-utils/src/ipc.rs @@ -184,8 +184,11 @@ where } /// Receives an event, along with the current [`ErrorContext`], on this [`IpcReceiverWithContext`]'s socket. - pub fn recv(&mut self) -> (T, ErrorContext) { - bincode::deserialize_from(&mut self.receiver).unwrap() + pub fn recv(&mut self) -> Option<(T, ErrorContext)> { + match bincode::deserialize_from(&mut self.receiver) { + Ok(msg) => Some(msg), + Err(_) => None, + } } /// Returns an [`IpcSenderWithContext`] with the same socket as this receiver.