From e07734941419b2df2263fde00cd7519a790c4009 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Sat, 15 Apr 2023 10:13:33 +0200 Subject: [PATCH 1/2] Wait for encoder to shut down before shutting down the other threads --- crates/re_sdk_comms/src/buffered_client.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/re_sdk_comms/src/buffered_client.rs b/crates/re_sdk_comms/src/buffered_client.rs index ed6a389fde1a..e0a66749cc62 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."); From 3d16793695bb8717eb80b36c9ca89db16db93643 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Sat, 15 Apr 2023 10:13:43 +0200 Subject: [PATCH 2/2] Remove TODOs and clean up the code --- crates/re_sdk_comms/src/buffered_client.rs | 25 +++++++--------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/crates/re_sdk_comms/src/buffered_client.rs b/crates/re_sdk_comms/src/buffered_client.rs index e0a66749cc62..797a4a23b34e 100644 --- a/crates/re_sdk_comms/src/buffered_client.rs +++ b/crates/re_sdk_comms/src/buffered_client.rs @@ -198,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 }