From 69bf801c168432cb150e2a116c0087cc8c23436f Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Sat, 15 Apr 2023 19:14:28 +0200 Subject: [PATCH] Fix shutdown race condition in `re_sdk_comms` client (#1861) * Wait for encoder to shut down before shutting down the other threads --- crates/re_sdk_comms/src/buffered_client.rs | 29 ++++++++-------------- 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/crates/re_sdk_comms/src/buffered_client.rs b/crates/re_sdk_comms/src/buffered_client.rs index ed6a389fde1a..797a4a23b34e 100644 --- a/crates/re_sdk_comms/src/buffered_client.rs +++ b/crates/re_sdk_comms/src/buffered_client.rs @@ -148,10 +148,12 @@ impl Drop for Client { re_log::debug!("Shutting down the client connection…"); self.send(LogMsg::Goodbye(RowId::random())); self.flush(); + // First shut down the encoder: self.encode_quit_tx.send(QuitMsg).ok(); + self.encode_join.take().map(|j| j.join().ok()); + // Then the other threads: self.send_quit_tx.send(InterruptMsg::Quit).ok(); self.drop_quit_tx.send(QuitMsg).ok(); - self.encode_join.take().map(|j| j.join().ok()); self.send_join.take().map(|j| j.join().ok()); self.drop_join.take().map(|j| j.join().ok()); re_log::debug!("TCP client has shut down."); @@ -196,23 +198,14 @@ fn msg_encode( MsgMsg::Flush => PacketMsg::Flush, }; - // TODO(jleibs): It's not clear why we're hitting this case, but an error here is still better than - // a panic. See: https://github.com/rerun-io/rerun/issues/1855 - match packet_tx.send(packet_msg) { - Ok(_) => {}, - Err(_) => { - re_log::error!("Failed to send message to tcp_sender thread. Likely a shutdown race-condition."); - }, - }; - - // TODO(jleibs): It's not clear why we're hitting this case, but an error here is still better than - // a panic. See: https://github.com/rerun-io/rerun/issues/1855 - match msg_drop_tx.send(msg_msg) { - Ok(_) => {}, - Err(_) => { - re_log::error!("Failed to send message to msg_dropp thread. Likely a shutdown race-condition"); - }, - }; + if packet_tx.send(packet_msg).is_err() { + re_log::error!("Failed to send message to tcp_sender thread. Likely a shutdown race-condition."); + return; + } + if msg_drop_tx.send(msg_msg).is_err() { + re_log::error!("Failed to send message to msg_drop thread. Likely a shutdown race-condition"); + return; + } } else { return; // channel has closed }