From c784dfe125ef7ca026ff260d18a860f4484166e8 Mon Sep 17 00:00:00 2001 From: Carles Fernandez Date: Sat, 27 Jun 2020 09:52:59 +0200 Subject: [PATCH] Fix a bug that could cause a crash on receiver stopping If a channel event was happening after flowgraph stop and before flowgraph disconnection, it caused a crash. This was avoided by sleeping the control thread during 500 ms after disconnection and before the block destructors were called, so the event could be processed, but this was not a robust solution. --- .../adapters/beidou_b1i_pcps_acquisition.cc | 1 + .../adapters/beidou_b3i_pcps_acquisition.cc | 1 + ...lileo_e1_pcps_8ms_ambiguous_acquisition.cc | 1 + .../galileo_e1_pcps_ambiguous_acquisition.cc | 1 + ...eo_e1_pcps_cccwsr_ambiguous_acquisition.cc | 2 ++ ...e1_pcps_quicksync_ambiguous_acquisition.cc | 2 ++ ...ileo_e1_pcps_tong_ambiguous_acquisition.cc | 2 ++ ...ileo_e5a_noncoherent_iq_acquisition_caf.cc | 2 ++ .../adapters/galileo_e5a_pcps_acquisition.cc | 1 + .../glonass_l1_ca_pcps_acquisition.cc | 1 + .../glonass_l2_ca_pcps_acquisition.cc | 1 + .../adapters/gps_l1_ca_pcps_acquisition.cc | 1 + ...gps_l1_ca_pcps_acquisition_fine_doppler.cc | 2 ++ .../gps_l1_ca_pcps_assisted_acquisition.cc | 2 ++ .../gps_l1_ca_pcps_opencl_acquisition.cc | 2 ++ .../gps_l1_ca_pcps_quicksync_acquisition.cc | 2 ++ .../gps_l1_ca_pcps_tong_acquisition.cc | 2 ++ .../adapters/gps_l2_m_pcps_acquisition.cc | 1 + .../adapters/gps_l5i_pcps_acquisition.cc | 1 + src/algorithms/channel/adapters/channel.h | 2 +- src/core/receiver/control_thread.cc | 14 +++++++--- src/core/receiver/control_thread.h | 27 ++++++++++--------- src/core/receiver/gnss_flowgraph.cc | 4 +++ .../control-plane/control_thread_test.cc | 2 ++ 24 files changed, 59 insertions(+), 18 deletions(-) diff --git a/src/algorithms/acquisition/adapters/beidou_b1i_pcps_acquisition.cc b/src/algorithms/acquisition/adapters/beidou_b1i_pcps_acquisition.cc index a0e6646c4..e930fba94 100644 --- a/src/algorithms/acquisition/adapters/beidou_b1i_pcps_acquisition.cc +++ b/src/algorithms/acquisition/adapters/beidou_b1i_pcps_acquisition.cc @@ -94,6 +94,7 @@ BeidouB1iPcpsAcquisition::BeidouB1iPcpsAcquisition( void BeidouB1iPcpsAcquisition::stop_acquisition() { + acquisition_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/beidou_b3i_pcps_acquisition.cc b/src/algorithms/acquisition/adapters/beidou_b3i_pcps_acquisition.cc index c5b614f39..599ea00e8 100644 --- a/src/algorithms/acquisition/adapters/beidou_b3i_pcps_acquisition.cc +++ b/src/algorithms/acquisition/adapters/beidou_b3i_pcps_acquisition.cc @@ -92,6 +92,7 @@ BeidouB3iPcpsAcquisition::BeidouB3iPcpsAcquisition( void BeidouB3iPcpsAcquisition::stop_acquisition() { + acquisition_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/galileo_e1_pcps_8ms_ambiguous_acquisition.cc b/src/algorithms/acquisition/adapters/galileo_e1_pcps_8ms_ambiguous_acquisition.cc index f4386e1c3..73ab611a3 100644 --- a/src/algorithms/acquisition/adapters/galileo_e1_pcps_8ms_ambiguous_acquisition.cc +++ b/src/algorithms/acquisition/adapters/galileo_e1_pcps_8ms_ambiguous_acquisition.cc @@ -121,6 +121,7 @@ GalileoE1Pcps8msAmbiguousAcquisition::GalileoE1Pcps8msAmbiguousAcquisition( void GalileoE1Pcps8msAmbiguousAcquisition::stop_acquisition() { + acquisition_cc_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/galileo_e1_pcps_ambiguous_acquisition.cc b/src/algorithms/acquisition/adapters/galileo_e1_pcps_ambiguous_acquisition.cc index d625c9fb4..afdc52046 100644 --- a/src/algorithms/acquisition/adapters/galileo_e1_pcps_ambiguous_acquisition.cc +++ b/src/algorithms/acquisition/adapters/galileo_e1_pcps_ambiguous_acquisition.cc @@ -94,6 +94,7 @@ GalileoE1PcpsAmbiguousAcquisition::GalileoE1PcpsAmbiguousAcquisition( void GalileoE1PcpsAmbiguousAcquisition::stop_acquisition() { + acquisition_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/galileo_e1_pcps_cccwsr_ambiguous_acquisition.cc b/src/algorithms/acquisition/adapters/galileo_e1_pcps_cccwsr_ambiguous_acquisition.cc index b6afd4e2a..76e0beb40 100644 --- a/src/algorithms/acquisition/adapters/galileo_e1_pcps_cccwsr_ambiguous_acquisition.cc +++ b/src/algorithms/acquisition/adapters/galileo_e1_pcps_cccwsr_ambiguous_acquisition.cc @@ -114,6 +114,8 @@ GalileoE1PcpsCccwsrAmbiguousAcquisition::GalileoE1PcpsCccwsrAmbiguousAcquisition void GalileoE1PcpsCccwsrAmbiguousAcquisition::stop_acquisition() { + acquisition_cc_->set_state(0); + acquisition_cc_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/galileo_e1_pcps_quicksync_ambiguous_acquisition.cc b/src/algorithms/acquisition/adapters/galileo_e1_pcps_quicksync_ambiguous_acquisition.cc index e0ae0ccdb..7d5a8cfed 100644 --- a/src/algorithms/acquisition/adapters/galileo_e1_pcps_quicksync_ambiguous_acquisition.cc +++ b/src/algorithms/acquisition/adapters/galileo_e1_pcps_quicksync_ambiguous_acquisition.cc @@ -155,6 +155,8 @@ GalileoE1PcpsQuickSyncAmbiguousAcquisition::GalileoE1PcpsQuickSyncAmbiguousAcqui void GalileoE1PcpsQuickSyncAmbiguousAcquisition::stop_acquisition() { + acquisition_cc_->set_state(0); + acquisition_cc_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/galileo_e1_pcps_tong_ambiguous_acquisition.cc b/src/algorithms/acquisition/adapters/galileo_e1_pcps_tong_ambiguous_acquisition.cc index 37a844970..90b35f9a3 100644 --- a/src/algorithms/acquisition/adapters/galileo_e1_pcps_tong_ambiguous_acquisition.cc +++ b/src/algorithms/acquisition/adapters/galileo_e1_pcps_tong_ambiguous_acquisition.cc @@ -125,6 +125,8 @@ GalileoE1PcpsTongAmbiguousAcquisition::GalileoE1PcpsTongAmbiguousAcquisition( void GalileoE1PcpsTongAmbiguousAcquisition::stop_acquisition() { + acquisition_cc_->set_state(0); + acquisition_cc_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/galileo_e5a_noncoherent_iq_acquisition_caf.cc b/src/algorithms/acquisition/adapters/galileo_e5a_noncoherent_iq_acquisition_caf.cc index 1d9d0a72f..0a415441e 100644 --- a/src/algorithms/acquisition/adapters/galileo_e5a_noncoherent_iq_acquisition_caf.cc +++ b/src/algorithms/acquisition/adapters/galileo_e5a_noncoherent_iq_acquisition_caf.cc @@ -130,6 +130,8 @@ GalileoE5aNoncoherentIQAcquisitionCaf::GalileoE5aNoncoherentIQAcquisitionCaf( void GalileoE5aNoncoherentIQAcquisitionCaf::stop_acquisition() { + acquisition_cc_->set_state(0); + acquisition_cc_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/galileo_e5a_pcps_acquisition.cc b/src/algorithms/acquisition/adapters/galileo_e5a_pcps_acquisition.cc index b9ffb1a82..785e3500f 100644 --- a/src/algorithms/acquisition/adapters/galileo_e5a_pcps_acquisition.cc +++ b/src/algorithms/acquisition/adapters/galileo_e5a_pcps_acquisition.cc @@ -93,6 +93,7 @@ GalileoE5aPcpsAcquisition::GalileoE5aPcpsAcquisition(ConfigurationInterface* con void GalileoE5aPcpsAcquisition::stop_acquisition() { + acquisition_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/glonass_l1_ca_pcps_acquisition.cc b/src/algorithms/acquisition/adapters/glonass_l1_ca_pcps_acquisition.cc index fd9a93a5a..7e0a28e5b 100644 --- a/src/algorithms/acquisition/adapters/glonass_l1_ca_pcps_acquisition.cc +++ b/src/algorithms/acquisition/adapters/glonass_l1_ca_pcps_acquisition.cc @@ -94,6 +94,7 @@ GlonassL1CaPcpsAcquisition::GlonassL1CaPcpsAcquisition( void GlonassL1CaPcpsAcquisition::stop_acquisition() { + acquisition_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/glonass_l2_ca_pcps_acquisition.cc b/src/algorithms/acquisition/adapters/glonass_l2_ca_pcps_acquisition.cc index 35e43680c..cf01ba49b 100644 --- a/src/algorithms/acquisition/adapters/glonass_l2_ca_pcps_acquisition.cc +++ b/src/algorithms/acquisition/adapters/glonass_l2_ca_pcps_acquisition.cc @@ -93,6 +93,7 @@ GlonassL2CaPcpsAcquisition::GlonassL2CaPcpsAcquisition( void GlonassL2CaPcpsAcquisition::stop_acquisition() { + acquisition_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_acquisition.cc b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_acquisition.cc index 2f8a11150..c72471cc6 100644 --- a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_acquisition.cc +++ b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_acquisition.cc @@ -96,6 +96,7 @@ GpsL1CaPcpsAcquisition::GpsL1CaPcpsAcquisition( void GpsL1CaPcpsAcquisition::stop_acquisition() { + acquisition_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_acquisition_fine_doppler.cc b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_acquisition_fine_doppler.cc index a9f41c5f0..02eedf4ed 100644 --- a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_acquisition_fine_doppler.cc +++ b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_acquisition_fine_doppler.cc @@ -100,6 +100,8 @@ GpsL1CaPcpsAcquisitionFineDoppler::GpsL1CaPcpsAcquisitionFineDoppler( void GpsL1CaPcpsAcquisitionFineDoppler::stop_acquisition() { + acquisition_cc_->set_state(0); + acquisition_cc_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_assisted_acquisition.cc b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_assisted_acquisition.cc index ed79ecb52..8f55eabaf 100644 --- a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_assisted_acquisition.cc +++ b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_assisted_acquisition.cc @@ -92,6 +92,8 @@ GpsL1CaPcpsAssistedAcquisition::GpsL1CaPcpsAssistedAcquisition( void GpsL1CaPcpsAssistedAcquisition::stop_acquisition() { + acquisition_cc_->set_active(false); + acquisition_cc_->set_state(0); } diff --git a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_opencl_acquisition.cc b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_opencl_acquisition.cc index 2d0bc316c..c19f64295 100644 --- a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_opencl_acquisition.cc +++ b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_opencl_acquisition.cc @@ -119,6 +119,8 @@ GpsL1CaPcpsOpenClAcquisition::GpsL1CaPcpsOpenClAcquisition( void GpsL1CaPcpsOpenClAcquisition::stop_acquisition() { + acquisition_cc_->set_active(false); + acquisition_cc_->set_state(0); } diff --git a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_quicksync_acquisition.cc b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_quicksync_acquisition.cc index 61a40ab9c..27a7dcf2f 100644 --- a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_quicksync_acquisition.cc +++ b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_quicksync_acquisition.cc @@ -148,6 +148,8 @@ GpsL1CaPcpsQuickSyncAcquisition::GpsL1CaPcpsQuickSyncAcquisition( void GpsL1CaPcpsQuickSyncAcquisition::stop_acquisition() { + acquisition_cc_->set_state(0); + acquisition_cc_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_tong_acquisition.cc b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_tong_acquisition.cc index eb742c513..27e01387a 100644 --- a/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_tong_acquisition.cc +++ b/src/algorithms/acquisition/adapters/gps_l1_ca_pcps_tong_acquisition.cc @@ -110,6 +110,8 @@ GpsL1CaPcpsTongAcquisition::GpsL1CaPcpsTongAcquisition( void GpsL1CaPcpsTongAcquisition::stop_acquisition() { + acquisition_cc_->set_state(0); + acquisition_cc_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/gps_l2_m_pcps_acquisition.cc b/src/algorithms/acquisition/adapters/gps_l2_m_pcps_acquisition.cc index 720cdccb2..d5e61e8be 100644 --- a/src/algorithms/acquisition/adapters/gps_l2_m_pcps_acquisition.cc +++ b/src/algorithms/acquisition/adapters/gps_l2_m_pcps_acquisition.cc @@ -93,6 +93,7 @@ GpsL2MPcpsAcquisition::GpsL2MPcpsAcquisition( void GpsL2MPcpsAcquisition::stop_acquisition() { + acquisition_->set_active(false); } diff --git a/src/algorithms/acquisition/adapters/gps_l5i_pcps_acquisition.cc b/src/algorithms/acquisition/adapters/gps_l5i_pcps_acquisition.cc index e893d21a2..cfbb0f685 100644 --- a/src/algorithms/acquisition/adapters/gps_l5i_pcps_acquisition.cc +++ b/src/algorithms/acquisition/adapters/gps_l5i_pcps_acquisition.cc @@ -94,6 +94,7 @@ GpsL5iPcpsAcquisition::GpsL5iPcpsAcquisition( void GpsL5iPcpsAcquisition::stop_acquisition() { + acquisition_->set_active(false); } diff --git a/src/algorithms/channel/adapters/channel.h b/src/algorithms/channel/adapters/channel.h index 698e602ca..1f7680aa7 100644 --- a/src/algorithms/channel/adapters/channel.h +++ b/src/algorithms/channel/adapters/channel.h @@ -84,10 +84,10 @@ public: void msg_handler_events(pmt::pmt_t msg); private: + std::shared_ptr channel_fsm_; std::shared_ptr acq_; std::shared_ptr trk_; std::shared_ptr nav_; - std::shared_ptr channel_fsm_; channel_msg_receiver_cc_sptr channel_msg_rx_; Concurrent_Queue* queue_; Gnss_Synchro gnss_synchro_{}; diff --git a/src/core/receiver/control_thread.cc b/src/core/receiver/control_thread.cc index 15730aff6..e45451634 100644 --- a/src/core/receiver/control_thread.cc +++ b/src/core/receiver/control_thread.cc @@ -102,6 +102,7 @@ ControlThread::ControlThread(std::shared_ptr configurati void ControlThread::init() { + telecommand_enabled_ = configuration_->property("GNSS-SDR.telecommand_enabled", false); // OPTIONAL: specify a custom year to override the system time in order to postprocess old gnss records and avoid wrong week rollover pre_2009_file_ = configuration_->property("GNSS-SDR.pre_2009_file", false); // Instantiates a control queue, a GNSS flowgraph, and a control message factory @@ -208,15 +209,14 @@ ControlThread::~ControlThread() // NOLINT(modernize-use-equals-default) if (cmd_interface_thread_.joinable()) { - cmd_interface_thread_.detach(); + cmd_interface_thread_.join(); } } void ControlThread::telecommand_listener() { - bool telecommand_enabled = configuration_->property("GNSS-SDR.telecommand_enabled", false); - if (telecommand_enabled) + if (telecommand_enabled_) { int tcp_cmd_port = configuration_->property("GNSS-SDR.telecommand_tcp_port", 3333); cmd_interface_.run_cmd_server(tcp_cmd_port); @@ -356,12 +356,18 @@ int ControlThread::run() fpga_helper_thread_.try_join_until(boost::chrono::steady_clock::now() + boost::chrono::milliseconds(1000)); #endif - std::this_thread::sleep_for(std::chrono::milliseconds(500)); // Terminate keyboard thread pthread_t id = keyboard_thread_.native_handle(); keyboard_thread_.detach(); pthread_cancel(id); + if (telecommand_enabled_) + { + pthread_t id2 = cmd_interface_thread_.native_handle(); + cmd_interface_thread_.detach(); + pthread_cancel(id2); + } + LOG(INFO) << "Flowgraph stopped"; if (restart_) diff --git a/src/core/receiver/control_thread.h b/src/core/receiver/control_thread.h index d2a435dfe..cbff8fd9d 100644 --- a/src/core/receiver/control_thread.h +++ b/src/core/receiver/control_thread.h @@ -147,19 +147,6 @@ private: void keyboard_listener(); void sysv_queue_listener(); - std::shared_ptr configuration_; - std::shared_ptr> control_queue_; - std::shared_ptr flowgraph_; - - std::thread cmd_interface_thread_; - std::thread keyboard_thread_; - std::thread sysv_queue_thread_; - std::thread gps_acq_assist_data_collector_thread_; - -#ifdef ENABLE_FPGA - boost::thread fpga_helper_thread_; -#endif - // default filename for assistance data const std::string eph_default_xml_filename_ = "./gps_ephemeris.xml"; const std::string utc_default_xml_filename_ = "./gps_utc_model.xml"; @@ -176,6 +163,19 @@ private: const std::string gal_almanac_default_xml_filename_ = "./gal_almanac.xml"; const std::string gps_almanac_default_xml_filename_ = "./gps_almanac.xml"; + std::shared_ptr configuration_; + std::shared_ptr> control_queue_; + std::shared_ptr flowgraph_; + + std::thread cmd_interface_thread_; + std::thread keyboard_thread_; + std::thread sysv_queue_thread_; + std::thread gps_acq_assist_data_collector_thread_; + +#ifdef ENABLE_FPGA + boost::thread fpga_helper_thread_; +#endif + TcpCmdInterface cmd_interface_; // SUPL assistance classes @@ -196,6 +196,7 @@ private: bool receiver_on_standby_; bool stop_; bool restart_; + bool telecommand_enabled_; bool pre_2009_file_; // to override the system time to postprocess old gnss records and avoid wrong week rollover }; diff --git a/src/core/receiver/gnss_flowgraph.cc b/src/core/receiver/gnss_flowgraph.cc index 98ac9c239..9e9b1b3cc 100644 --- a/src/core/receiver/gnss_flowgraph.cc +++ b/src/core/receiver/gnss_flowgraph.cc @@ -114,6 +114,10 @@ void GNSSFlowgraph::start() void GNSSFlowgraph::stop() { + for (auto chan : channels_) + { + chan->stop_channel(); // stop the acquisition or tracking operation + } top_block_->stop(); running_ = false; } diff --git a/src/tests/unit-tests/control-plane/control_thread_test.cc b/src/tests/unit-tests/control-plane/control_thread_test.cc index 33d19c6f8..5f7a99e0b 100644 --- a/src/tests/unit-tests/control-plane/control_thread_test.cc +++ b/src/tests/unit-tests/control-plane/control_thread_test.cc @@ -135,6 +135,7 @@ TEST_F(ControlThreadTest /*unused*/, InstantiateRunControlMessages /*unused*/) unsigned int expected1 = 1; EXPECT_EQ(expected3, control_thread->processed_control_messages()); EXPECT_EQ(expected1, control_thread->applied_actions()); + std::this_thread::sleep_for(std::chrono::milliseconds(500)); } @@ -196,6 +197,7 @@ TEST_F(ControlThreadTest /*unused*/, InstantiateRunControlMessages2 /*unused*/) unsigned int expected1 = 1; EXPECT_EQ(expected5, control_thread2->processed_control_messages()); EXPECT_EQ(expected1, control_thread2->applied_actions()); + std::this_thread::sleep_for(std::chrono::milliseconds(500)); }