From 431f788a76c3ee74fabc888dfe4804e657c64f0a Mon Sep 17 00:00:00 2001 From: Vladslav P Date: Wed, 2 Sep 2026 05:31:37 +0300 Subject: [PATCH 1/4] gnss_synchro_monitor: fix decimation Consuming input samples only when the decimation counter reaches it's limit prevents decimation from working correctly. It just changes one-by-one output to batch output and attempts to decimate batches instead of individual synchos. Consume unconditionally in every outer loop iteration and change loop nesting to make sure, that all channels will be decimated equally and no batches will be formed. Add some safty checks against read beyond the end of the buffer and empty array submission to Gnss_Synchro_Udp_Sink. Signed-off-by: Vladslav P --- src/core/monitor/gnss_synchro_monitor.cc | 42 +++++++++++++++++------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/src/core/monitor/gnss_synchro_monitor.cc b/src/core/monitor/gnss_synchro_monitor.cc index f63811a73..5f1aefce7 100644 --- a/src/core/monitor/gnss_synchro_monitor.cc +++ b/src/core/monitor/gnss_synchro_monitor.cc @@ -68,27 +68,45 @@ int gnss_synchro_monitor::general_work(int noutput_items __attribute__((unused)) { // Get the input buffer pointer const auto** in = reinterpret_cast(&input_items[0]); + // Convert to a vector and write to the UDP sink + std::vector stocks; - // Loop through each input stream channel + // Find maximum number of items across of all inputs + int n_items_max = 0; for (int channel_index = 0; channel_index < d_nchannels; channel_index++) { - // Loop through each item in each input stream channel - for (int item_index = 0; item_index < ninput_items[channel_index]; item_index++) + n_items_max = std::max(n_items_max, ninput_items[channel_index]); + } + + // Loop through each item in each input stream channel + for (int item_index = 0; item_index < n_items_max; item_index++) + { + // Use the count variable to limit how many items are sent per channel + count++; + // Loop through each input stream channel + for (int channel_index = 0; channel_index < d_nchannels; channel_index++) { - // Use the count variable to limit how many items are sent per channel - count++; + if (item_index >= ninput_items[channel_index]) + { + continue; + } if (count >= d_decimation_factor) { - // Convert to a vector and write to the UDP sink - std::vector stocks; stocks.push_back(in[channel_index][item_index]); - udp_sink_ptr->write_gnss_synchro(stocks); - // Reset count variable - count = 0; - // Consume the number of items for the input stream channel - consume(channel_index, ninput_items[channel_index]); } + // Consume the number of items for the input stream channel + consume(channel_index, 1); } + if (count >= d_decimation_factor) + { + // Reset count variable + count = 0; + } + } + // Do not send empty datagrams + if (!stocks.empty()) + { + udp_sink_ptr->write_gnss_synchro(stocks); } // Not producing any outputs From 32911d38944d513c40b64d25274c7e89323753f0 Mon Sep 17 00:00:00 2001 From: Vladslav P Date: Thu, 3 Sep 2026 06:18:29 +0300 Subject: [PATCH 2/4] Fix run_tests random crash during termination Implement proper protobuf library cleanup mechanism. This prevents use after free and hard to debug crashes. --- src/algorithms/PVT/libs/CMakeLists.txt | 1 + src/algorithms/PVT/libs/serdes_galileo_eph.h | 8 ++- src/algorithms/PVT/libs/serdes_gps_eph.h | 8 ++- src/algorithms/PVT/libs/serdes_monitor_pvt.h | 8 ++- src/core/libs/CMakeLists.txt | 1 + src/core/libs/protobuf_cleanup_manager.h | 55 ++++++++++++++++++++ src/core/libs/serdes_nav_message.h | 8 ++- src/core/monitor/CMakeLists.txt | 1 + src/core/monitor/serdes_gnss_synchro.h | 8 ++- 9 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 src/core/libs/protobuf_cleanup_manager.h diff --git a/src/algorithms/PVT/libs/CMakeLists.txt b/src/algorithms/PVT/libs/CMakeLists.txt index d610d7e24..5ad3e11e1 100644 --- a/src/algorithms/PVT/libs/CMakeLists.txt +++ b/src/algorithms/PVT/libs/CMakeLists.txt @@ -120,6 +120,7 @@ get_filename_component(PROTO_INCLUDE_HEADERS_DIR ${PROTO_HDRS} DIRECTORY) target_include_directories(pvt_libs PUBLIC ${GNSSSDR_SOURCE_DIR}/src/core/receiver + ${GNSSSDR_SOURCE_DIR}/src/core/libs ) # Do not apply clang-tidy fixes to protobuf generated headers diff --git a/src/algorithms/PVT/libs/serdes_galileo_eph.h b/src/algorithms/PVT/libs/serdes_galileo_eph.h index f61be59e7..47b5cedbd 100644 --- a/src/algorithms/PVT/libs/serdes_galileo_eph.h +++ b/src/algorithms/PVT/libs/serdes_galileo_eph.h @@ -20,6 +20,7 @@ #include "galileo_ephemeris.h" #include "galileo_ephemeris.pb.h" // file created by Protocol Buffers at compile time +#include "protobuf_cleanup_manager.h" #include #include #include @@ -42,11 +43,16 @@ public: // Verify that the version of the library that we linked against is // compatible with the version of the headers we compiled against. GOOGLE_PROTOBUF_VERIFY_VERSION; + + // This is a static instance of "protobuf resource cleaner" class, + // that would be destructed only once at the end of the program execution + // This prevents random crashes due to memory corruption + // And keeps memory leak checkers happy + volatile auto& protobuf_cleaner __attribute__((unused)) = protobuf_cleanup_manager::get(); } ~Serdes_Galileo_Eph() { - // google::protobuf::ShutdownProtobufLibrary(); } inline Serdes_Galileo_Eph(const Serdes_Galileo_Eph& other) noexcept : monitor_(other.monitor_) //!< Copy constructor diff --git a/src/algorithms/PVT/libs/serdes_gps_eph.h b/src/algorithms/PVT/libs/serdes_gps_eph.h index d0f506686..ae3aeea6d 100644 --- a/src/algorithms/PVT/libs/serdes_gps_eph.h +++ b/src/algorithms/PVT/libs/serdes_gps_eph.h @@ -20,6 +20,7 @@ #include "gps_ephemeris.h" #include "gps_ephemeris.pb.h" // file created by Protocol Buffers at compile time +#include "protobuf_cleanup_manager.h" #include #include #include @@ -41,11 +42,16 @@ public: // Verify that the version of the library that we linked against is // compatible with the version of the headers we compiled against. GOOGLE_PROTOBUF_VERIFY_VERSION; + + // This is a static instance of "protobuf resource cleaner" class, + // that would be destructed only once at the end of the program execution + // This prevents random crashes due to memory corruption + // And keeps memory leak checkers happy + volatile auto& protobuf_cleaner __attribute__((unused)) = protobuf_cleanup_manager::get(); } ~Serdes_Gps_Eph() { - // google::protobuf::ShutdownProtobufLibrary(); } inline Serdes_Gps_Eph(const Serdes_Gps_Eph& other) noexcept : monitor_(other.monitor_) //!< Copy constructor diff --git a/src/algorithms/PVT/libs/serdes_monitor_pvt.h b/src/algorithms/PVT/libs/serdes_monitor_pvt.h index f6d0b15e4..e5c9997bb 100644 --- a/src/algorithms/PVT/libs/serdes_monitor_pvt.h +++ b/src/algorithms/PVT/libs/serdes_monitor_pvt.h @@ -20,6 +20,7 @@ #include "monitor_pvt.h" #include "monitor_pvt.pb.h" // file created by Protocol Buffers at compile time +#include "protobuf_cleanup_manager.h" #include #include #include @@ -42,11 +43,16 @@ public: // Verify that the version of the library that we linked against is // compatible with the version of the headers we compiled against. GOOGLE_PROTOBUF_VERIFY_VERSION; + + // This is a static instance of "protobuf resource cleaner" class, + // that would be destructed only once at the end of the program execution + // This prevents random crashes due to memory corruption + // And keeps memory leak checkers happy + volatile auto& protobuf_cleaner __attribute__((unused)) = protobuf_cleanup_manager::get(); } ~Serdes_Monitor_Pvt() { - // google::protobuf::ShutdownProtobufLibrary(); } inline Serdes_Monitor_Pvt(const Serdes_Monitor_Pvt& other) noexcept : monitor_(other.monitor_) //!< Copy constructor diff --git a/src/core/libs/CMakeLists.txt b/src/core/libs/CMakeLists.txt index d0f5bdd2a..75dfa4f0c 100644 --- a/src/core/libs/CMakeLists.txt +++ b/src/core/libs/CMakeLists.txt @@ -45,6 +45,7 @@ set(CORE_LIBS_HEADERS osnma_helper.h osnma_msg_receiver.h osnma_nav_data_manager.h + protobuf_cleanup_manager.h serdes_nav_message.h string_converter.h ) diff --git a/src/core/libs/protobuf_cleanup_manager.h b/src/core/libs/protobuf_cleanup_manager.h new file mode 100644 index 000000000..b38b4431c --- /dev/null +++ b/src/core/libs/protobuf_cleanup_manager.h @@ -0,0 +1,55 @@ +/*! + * \file protobuf_cleanup_manager.h + * \brief Singleton class, that implements shared resourse cleaner pattern for + * Protocol Buffers library + * \author Vladislav P, 2026. vladisslav2011(at)gmail.com + * + * ----------------------------------------------------------------------------- + * + * GNSS-SDR is a Global Navigation Satellite System software-defined receiver. + * This file is part of GNSS-SDR. + * + * Copyright (C) 2010-2020 (see AUTHORS file for a list of contributors) + * SPDX-License-Identifier: GPL-3.0-or-later + * + * ----------------------------------------------------------------------------- + */ + +#ifndef PROTOBUF_CLEANUP_MANAGER_H +#define PROTOBUF_CLEANUP_MANAGER_H + +#include +#include + +/** \addtogroup Core + * \{ */ +/** \addtogroup Core_Receiver_Library + * \{ */ + +/*! + * \brief This class implements protobuf library shared memory cleanup manager + * google::protobuf::ShutdownProtobufLibrary() should be called only once at the end of the program execution + * Calling google::protobuf::ShutdownProtobufLibrary() during tests execution results in a weird crash due to use-after-free + * Create an instance of this class in a constructor of every class, that uses protobuf library + * to make sure that google::protobuf::ShutdownProtobufLibrary() will be called at program shutdown. + */ +class protobuf_cleanup_manager +{ +public: + explicit protobuf_cleanup_manager() + { + } + ~protobuf_cleanup_manager() + { + google::protobuf::ShutdownProtobufLibrary(); + } + static protobuf_cleanup_manager& get() + { + static protobuf_cleanup_manager instance{}; + return instance; + } +}; + +/** \} */ +/** \} */ +#endif // PROTOBUF_CLEANUP_MANAGER_H diff --git a/src/core/libs/serdes_nav_message.h b/src/core/libs/serdes_nav_message.h index 4a7ada17c..c79b41f5c 100644 --- a/src/core/libs/serdes_nav_message.h +++ b/src/core/libs/serdes_nav_message.h @@ -20,6 +20,7 @@ #include "nav_message.pb.h" // file created by Protocol Buffers at compile time #include "nav_message_packet.h" +#include "protobuf_cleanup_manager.h" #include #include #include @@ -43,11 +44,16 @@ public: // Verify that the version of the library that we linked against is // compatible with the version of the headers we compiled against. GOOGLE_PROTOBUF_VERIFY_VERSION; + + // This is a static instance of "protobuf resource cleaner" class, + // that would be destructed only once at the end of the program execution + // This prevents test crashes due to memory corruption + // And keeps memory leak checkers happy + volatile auto& protobuf_cleaner __attribute__((unused)) = protobuf_cleanup_manager::get(); } ~Serdes_Nav_Message() { - // google::protobuf::ShutdownProtobufLibrary(); } inline Serdes_Nav_Message(const Serdes_Nav_Message& other) noexcept : navmsg_(other.navmsg_) //!< Copy constructor diff --git a/src/core/monitor/CMakeLists.txt b/src/core/monitor/CMakeLists.txt index 32369c4ae..2e9ec6759 100644 --- a/src/core/monitor/CMakeLists.txt +++ b/src/core/monitor/CMakeLists.txt @@ -59,6 +59,7 @@ target_include_directories(core_monitor ${GNSSSDR_SOURCE_DIR}/src/core/interfaces PRIVATE ${GNSSSDR_SOURCE_DIR}/src/algorithms/libs # for gnss_sdr_make_unique + ${GNSSSDR_SOURCE_DIR}/src/core/libs # for protobuf_cleanup_manager ) # Do not apply clang-tidy fixes to protobuf generated headers diff --git a/src/core/monitor/serdes_gnss_synchro.h b/src/core/monitor/serdes_gnss_synchro.h index 73af83208..a6e8e3eed 100644 --- a/src/core/monitor/serdes_gnss_synchro.h +++ b/src/core/monitor/serdes_gnss_synchro.h @@ -20,6 +20,7 @@ #include "gnss_synchro.h" #include "gnss_synchro.pb.h" // file created by Protocol Buffers at compile time +#include "protobuf_cleanup_manager.h" #include #include #include @@ -37,11 +38,16 @@ public: // Verify that the version of the library that we linked against is // compatible with the version of the headers we compiled against. GOOGLE_PROTOBUF_VERIFY_VERSION; + + // This is a static instance of "protobuf resource cleaner" class, + // that would be destructed only once at the end of the program execution + // This prevents random crashes due to memory corruption + // And keeps memory leak checkers happy + volatile auto& protobuf_cleaner __attribute__((unused)) = protobuf_cleanup_manager::get(); } ~Serdes_Gnss_Synchro() { - google::protobuf::ShutdownProtobufLibrary(); } inline Serdes_Gnss_Synchro(const Serdes_Gnss_Synchro& other) noexcept //!< Copy constructor From 0437bd8c659aaf65b2594cca7dd741d30fae3585 Mon Sep 17 00:00:00 2001 From: Vladslav P Date: Wed, 2 Sep 2026 05:35:41 +0300 Subject: [PATCH 3/4] Add gnss_syncro_monitor test Test basic gnss_syncro_monitor operation. Test decimation at different decimation factors. Signed-off-by: Vladslav P --- tests/test_main.cc | 1 + .../gnss_synchro_monitor_test.cc | 174 ++++++++++++++++++ 2 files changed, 175 insertions(+) create mode 100644 tests/unit-tests/control-plane/gnss_synchro_monitor_test.cc diff --git a/tests/test_main.cc b/tests/test_main.cc index e72cb9bb7..ca4befced 100644 --- a/tests/test_main.cc +++ b/tests/test_main.cc @@ -144,6 +144,7 @@ private: #include "unit-tests/arithmetic/magnitude_squared_test.cc" #include "unit-tests/arithmetic/multiply_test.cc" #include "unit-tests/arithmetic/preamble_correlator_test.cc" +#include "unit-tests/control-plane/gnss_synchro_monitor_test.cc" #include "unit-tests/control-plane/in_memory_configuration_test.cc" #include "unit-tests/control-plane/protobuf_test.cc" #include "unit-tests/control-plane/string_converter_test.cc" diff --git a/tests/unit-tests/control-plane/gnss_synchro_monitor_test.cc b/tests/unit-tests/control-plane/gnss_synchro_monitor_test.cc new file mode 100644 index 000000000..6b7d244b4 --- /dev/null +++ b/tests/unit-tests/control-plane/gnss_synchro_monitor_test.cc @@ -0,0 +1,174 @@ +/*! + * \file gnss_synchro_monitor_test.cc + * \brief This file implements tests for gnss_synchro_monitor + * \author Vladislav P, 2026. vladisslav2011(at)gmail.com + * + * ----------------------------------------------------------------------------- + * + * GNSS-SDR is a Global Navigation Satellite System software-defined receiver. + * This file is part of GNSS-SDR. + * + * Copyright (C) 2010-2020 (see AUTHORS file for a list of contributors) + * SPDX-License-Identifier: GPL-3.0-or-later + * + * ----------------------------------------------------------------------------- + */ + + +#include "gnss_block_interface.h" +#include "gnss_synchro_monitor.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include + +class gnss_synchro_emitter; + +using gnss_synchro_emitter_sptr = gnss_shared_ptr; + +class gnss_synchro_emitter : public gr::sync_block +{ +public: + virtual ~gnss_synchro_emitter() = default; + + // Emit syncros one by one with small pause to match actual "Monitor.enable_monitor=true" GNSS-SDR configuration behavior + int work(int /*noutput_items*/, gr_vector_const_void_star& /*input_items*/, gr_vector_void_star& output_items) + { + auto** out = reinterpret_cast(&output_items[0]); + if (counter >= d_nitems) + { + d_finished = true; + return WORK_DONE; + } + for (int i = 0; i < d_nchannels; i++) + { + out[i][0] = vgs[i]; + vgs[i].interp_TOW_ms++; + } + counter++; + usleep(d_interval_us); + return 1; + } + + void reset() + { + d_finished = false; + } + + bool finished() + { + return d_finished; + } + +private: + friend gnss_synchro_emitter_sptr gnss_synchro_make_emitter(int n_channels, int nitems, int interval_us); + + gnss_synchro_emitter(int n_channels, int nitems, int interval_us) : sync_block( + "gnss_synchro_emitter", gr::io_signature::make(0, 0, 0), gr::io_signature::make(n_channels, n_channels, sizeof(Gnss_Synchro))), + d_nitems(nitems), + d_nchannels(n_channels), + d_interval_us(interval_us) + { + for (int i = 0; i < d_nchannels; i++) + { + Gnss_Synchro gs{}; + // unique key: Channel_ID, interp_TOW_ms + gs.Channel_ID = i; + gs.interp_TOW_ms = 0; + // PRN is filled with a known value to validate deserialization result + gs.PRN = i + 1; + vgs.push_back(gs); + } + } + + const int d_nitems; + const int d_nchannels; + const int d_interval_us; + std::vector vgs; + int counter{0}; + bool d_finished{false}; +}; + +gnss_synchro_emitter_sptr gnss_synchro_make_emitter(int n_channels, int nitems, int interval_us) +{ + return gnss_synchro_emitter_sptr(new gnss_synchro_emitter(n_channels, nitems, interval_us)); +} + + +TEST(Monitor, decimation) +{ + // Minimum number of channels to confirm, that decimation by 2 is not performed just by dropping all synchros from one channel + constexpr uint32_t NCHANNELS = 2; + constexpr uint32_t NSYNCHRO = 16; + constexpr uint32_t NDEC_MAX = 8; + // Limit receive the test execution time + constexpr uint32_t RX_LOOP_MAX_ITER = 32; + constexpr uint32_t GNSS_SYNCHRO_INTERVAL_US = 1000; + b_io_context io_context; + boost::asio::ip::udp::socket socket(io_context, boost::asio::ip::udp::endpoint(boost::asio::ip::udp::v4(), 1112)); + std::vector recv_buf(8192); + boost::asio::ip::udp::endpoint remote_endpoint; + for (uint32_t ndec = 1; ndec <= NDEC_MAX; ndec <<= 1) + { + std::cout << "Testing decimation factor = " << ndec << "\n"; + const uint32_t nexpected = NSYNCHRO / ndec; + std::set expected{}; + uint32_t counter{0}; + for (uint32_t i = 0; i < NSYNCHRO; i++) + { + counter++; + if (counter >= ndec) + { + expected.insert(i); + counter = 0; + } + } + std::vector> received(NCHANNELS); + gnss_sdr::Observables obs{}; + Serdes_Gnss_Synchro serdes = Serdes_Gnss_Synchro(); + auto top_block = gr::make_top_block("monitor_test"); + auto emitter = gnss_synchro_make_emitter(NCHANNELS, NSYNCHRO, GNSS_SYNCHRO_INTERVAL_US); + auto monitor = gnss_synchro_make_monitor(NCHANNELS, ndec, {"1112"}, {"127.0.0.1"}, true); + for (uint32_t i = 0; i < NCHANNELS; i++) + { + top_block->connect(emitter, i, monitor, i); + } + top_block->start(); + for (uint32_t rx_iter = 0; rx_iter < RX_LOOP_MAX_ITER; rx_iter++) + { + usleep(GNSS_SYNCHRO_INTERVAL_US); + if (socket.available() == 0) + { + continue; + } + size_t nbytes = socket.receive_from(boost::asio::buffer(recv_buf), remote_endpoint); + EXPECT_GT(nbytes, 0); + obs.ParseFromArray(recv_buf.data(), nbytes); + const std::vector vgs_read = serdes.readProtobuffer(obs); + // validate deserialization result and add it's unique index to set + for (auto& gs : vgs_read) + { + EXPECT_EQ(gs.PRN, gs.Channel_ID + 1); + received[gs.Channel_ID].insert(gs.interp_TOW_ms); + } + // Everything received, exit loop + if (received[0].size() >= nexpected) + { + break; + } + } + top_block->stop(); + top_block->wait(); + // Check the number of unique syncros per channel against the expected number + for (auto& gs_set : received) + { + EXPECT_EQ(gs_set.size(), nexpected); + EXPECT_EQ(gs_set, expected); + } + } +} From 1ba1f37d3bdc8acf33885eaa9bfea5a9fafada25 Mon Sep 17 00:00:00 2001 From: Carles Fernandez Date: Sat, 5 Sep 2026 12:46:30 +0200 Subject: [PATCH 4/4] Polish the gnss_synchro_monitor decimation fix --- AUTHORS | 1 + CITATION.cff | 4 ++ docs/CHANGELOG.md | 9 +++ src/algorithms/PVT/libs/serdes_galileo_eph.h | 12 +--- src/algorithms/PVT/libs/serdes_gps_eph.h | 12 +--- src/algorithms/PVT/libs/serdes_monitor_pvt.h | 12 +--- src/core/libs/protobuf_cleanup_manager.h | 48 +++++++++------- src/core/libs/serdes_nav_message.h | 12 +--- src/core/monitor/CMakeLists.txt | 2 +- src/core/monitor/gnss_synchro_monitor.cc | 15 +++-- src/core/monitor/serdes_gnss_synchro.h | 12 +--- .../gnss_synchro_monitor_test.cc | 55 +++++++++---------- 12 files changed, 90 insertions(+), 104 deletions(-) diff --git a/AUTHORS b/AUTHORS index eac16f677..c88cdeaa3 100644 --- a/AUTHORS +++ b/AUTHORS @@ -77,6 +77,7 @@ Piyush Gupta piyush04111999@gmail.com Contributor Rodrigo Muñoz rodrigo.munoz@proteinlab.cl Contributor Stefan van der Linden spvdlinden@gmail.com Contributor Víctor Castillo-Agüero victorcastilloaguero@gmail.com Contributor +Vladislav P vladisslav2011@gmail.com Contributor Wenhao Ou ouwh@mail2.sysu.edu.cn Contributor Will Silberman wsilberm@google.com Contributor Carlos Paniego carpanie@hotmail.com Artwork diff --git a/CITATION.cff b/CITATION.cff index 2565ef994..e3eb3344d 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -108,6 +108,10 @@ authors: email: cillian.odriscoll@gmail.com family-names: "O'Driscoll" given-names: Cillian + - alias: vladisslav2011 + email: vladisslav2011@gmail.com + family-names: P + given-names: Vladislav - affiliation: "Finnish Geospatial Research Institute" alias: IntoPa email: into.paakkonen@aalto.fi diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 560b81026..7eff7e553 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -329,6 +329,15 @@ All notable changes to GNSS-SDR will be documented in this file. ### Improvements in Reliability: +- Fixed the decimation logic of the `Monitor`, `AcquisitionMonitor` and + `TrackingMonitor` blocks: `decimation_factor` now selects every N-th epoch and + always consumes all the input items, instead of grouping `Gnss_Synchro` + objects into bursts and skipping others, and empty datagrams are no longer + sent. Fixed a use-after-free memory corruption caused by + `google::protobuf::ShutdownProtobufLibrary()` being called from the destructor + of `Serdes_Gnss_Synchro`, before the protobuf library was actually used; the + library is now shut down only once, at program exit. Added a unit test for the + monitor decimation. Contributed by @vladisslav2011. - Hardened the Galileo OSNMA protocol implementation, adding support for Chain Renewal, Chain Revocation, Public Key Renewal, Public Key Revocation, Merkle Tree Renewal, and OSNMA Alert Message events. Improved the management of OSNMA diff --git a/src/algorithms/PVT/libs/serdes_galileo_eph.h b/src/algorithms/PVT/libs/serdes_galileo_eph.h index 47b5cedbd..880bc8035 100644 --- a/src/algorithms/PVT/libs/serdes_galileo_eph.h +++ b/src/algorithms/PVT/libs/serdes_galileo_eph.h @@ -44,15 +44,9 @@ public: // compatible with the version of the headers we compiled against. GOOGLE_PROTOBUF_VERIFY_VERSION; - // This is a static instance of "protobuf resource cleaner" class, - // that would be destructed only once at the end of the program execution - // This prevents random crashes due to memory corruption - // And keeps memory leak checkers happy - volatile auto& protobuf_cleaner __attribute__((unused)) = protobuf_cleanup_manager::get(); - } - - ~Serdes_Galileo_Eph() - { + // Make sure google::protobuf::ShutdownProtobufLibrary() is called only once, + // at the end of the program execution (see Protobuf_Cleanup_Manager) + Protobuf_Cleanup_Manager::get(); } inline Serdes_Galileo_Eph(const Serdes_Galileo_Eph& other) noexcept : monitor_(other.monitor_) //!< Copy constructor diff --git a/src/algorithms/PVT/libs/serdes_gps_eph.h b/src/algorithms/PVT/libs/serdes_gps_eph.h index ae3aeea6d..81d25e65d 100644 --- a/src/algorithms/PVT/libs/serdes_gps_eph.h +++ b/src/algorithms/PVT/libs/serdes_gps_eph.h @@ -43,15 +43,9 @@ public: // compatible with the version of the headers we compiled against. GOOGLE_PROTOBUF_VERIFY_VERSION; - // This is a static instance of "protobuf resource cleaner" class, - // that would be destructed only once at the end of the program execution - // This prevents random crashes due to memory corruption - // And keeps memory leak checkers happy - volatile auto& protobuf_cleaner __attribute__((unused)) = protobuf_cleanup_manager::get(); - } - - ~Serdes_Gps_Eph() - { + // Make sure google::protobuf::ShutdownProtobufLibrary() is called only once, + // at the end of the program execution (see Protobuf_Cleanup_Manager) + Protobuf_Cleanup_Manager::get(); } inline Serdes_Gps_Eph(const Serdes_Gps_Eph& other) noexcept : monitor_(other.monitor_) //!< Copy constructor diff --git a/src/algorithms/PVT/libs/serdes_monitor_pvt.h b/src/algorithms/PVT/libs/serdes_monitor_pvt.h index e5c9997bb..62f32220b 100644 --- a/src/algorithms/PVT/libs/serdes_monitor_pvt.h +++ b/src/algorithms/PVT/libs/serdes_monitor_pvt.h @@ -44,15 +44,9 @@ public: // compatible with the version of the headers we compiled against. GOOGLE_PROTOBUF_VERIFY_VERSION; - // This is a static instance of "protobuf resource cleaner" class, - // that would be destructed only once at the end of the program execution - // This prevents random crashes due to memory corruption - // And keeps memory leak checkers happy - volatile auto& protobuf_cleaner __attribute__((unused)) = protobuf_cleanup_manager::get(); - } - - ~Serdes_Monitor_Pvt() - { + // Make sure google::protobuf::ShutdownProtobufLibrary() is called only once, + // at the end of the program execution (see Protobuf_Cleanup_Manager) + Protobuf_Cleanup_Manager::get(); } inline Serdes_Monitor_Pvt(const Serdes_Monitor_Pvt& other) noexcept : monitor_(other.monitor_) //!< Copy constructor diff --git a/src/core/libs/protobuf_cleanup_manager.h b/src/core/libs/protobuf_cleanup_manager.h index b38b4431c..273813fa4 100644 --- a/src/core/libs/protobuf_cleanup_manager.h +++ b/src/core/libs/protobuf_cleanup_manager.h @@ -1,7 +1,7 @@ /*! * \file protobuf_cleanup_manager.h - * \brief Singleton class, that implements shared resourse cleaner pattern for - * Protocol Buffers library + * \brief Singleton class that implements the shared resource cleaner pattern + * for the Protocol Buffers library * \author Vladislav P, 2026. vladisslav2011(at)gmail.com * * ----------------------------------------------------------------------------- @@ -9,17 +9,16 @@ * GNSS-SDR is a Global Navigation Satellite System software-defined receiver. * This file is part of GNSS-SDR. * - * Copyright (C) 2010-2020 (see AUTHORS file for a list of contributors) + * Copyright (C) 2010-2026 (see AUTHORS file for a list of contributors) * SPDX-License-Identifier: GPL-3.0-or-later * * ----------------------------------------------------------------------------- */ -#ifndef PROTOBUF_CLEANUP_MANAGER_H -#define PROTOBUF_CLEANUP_MANAGER_H +#ifndef GNSS_SDR_PROTOBUF_CLEANUP_MANAGER_H +#define GNSS_SDR_PROTOBUF_CLEANUP_MANAGER_H #include -#include /** \addtogroup Core * \{ */ @@ -27,29 +26,38 @@ * \{ */ /*! - * \brief This class implements protobuf library shared memory cleanup manager - * google::protobuf::ShutdownProtobufLibrary() should be called only once at the end of the program execution - * Calling google::protobuf::ShutdownProtobufLibrary() during tests execution results in a weird crash due to use-after-free - * Create an instance of this class in a constructor of every class, that uses protobuf library - * to make sure that google::protobuf::ShutdownProtobufLibrary() will be called at program shutdown. + * \brief This class implements the Protocol Buffers library shared memory cleanup manager. + * + * google::protobuf::ShutdownProtobufLibrary() must be called only once, at the end + * of the program execution: it is not safe to use any other part of the library + * after that call. Calling it earlier (e.g., from the destructor of a class that + * uses protobuf) results in use-after-free memory corruption and random crashes. + * Call Protobuf_Cleanup_Manager::get() in the constructor of every class that + * uses the protobuf library, so that google::protobuf::ShutdownProtobufLibrary() + * is called at program shutdown, when the singleton is destroyed. */ -class protobuf_cleanup_manager +class Protobuf_Cleanup_Manager { public: - explicit protobuf_cleanup_manager() + static Protobuf_Cleanup_Manager& get() { + static Protobuf_Cleanup_Manager instance{}; + return instance; } - ~protobuf_cleanup_manager() + + Protobuf_Cleanup_Manager(const Protobuf_Cleanup_Manager&) = delete; + Protobuf_Cleanup_Manager& operator=(const Protobuf_Cleanup_Manager&) = delete; + Protobuf_Cleanup_Manager(Protobuf_Cleanup_Manager&&) = delete; + Protobuf_Cleanup_Manager& operator=(Protobuf_Cleanup_Manager&&) = delete; + +private: + Protobuf_Cleanup_Manager() = default; + ~Protobuf_Cleanup_Manager() { google::protobuf::ShutdownProtobufLibrary(); } - static protobuf_cleanup_manager& get() - { - static protobuf_cleanup_manager instance{}; - return instance; - } }; /** \} */ /** \} */ -#endif // PROTOBUF_CLEANUP_MANAGER_H +#endif // GNSS_SDR_PROTOBUF_CLEANUP_MANAGER_H diff --git a/src/core/libs/serdes_nav_message.h b/src/core/libs/serdes_nav_message.h index c79b41f5c..ff8b0b8d2 100644 --- a/src/core/libs/serdes_nav_message.h +++ b/src/core/libs/serdes_nav_message.h @@ -45,15 +45,9 @@ public: // compatible with the version of the headers we compiled against. GOOGLE_PROTOBUF_VERIFY_VERSION; - // This is a static instance of "protobuf resource cleaner" class, - // that would be destructed only once at the end of the program execution - // This prevents test crashes due to memory corruption - // And keeps memory leak checkers happy - volatile auto& protobuf_cleaner __attribute__((unused)) = protobuf_cleanup_manager::get(); - } - - ~Serdes_Nav_Message() - { + // Make sure google::protobuf::ShutdownProtobufLibrary() is called only once, + // at the end of the program execution (see Protobuf_Cleanup_Manager) + Protobuf_Cleanup_Manager::get(); } inline Serdes_Nav_Message(const Serdes_Nav_Message& other) noexcept : navmsg_(other.navmsg_) //!< Copy constructor diff --git a/src/core/monitor/CMakeLists.txt b/src/core/monitor/CMakeLists.txt index 2e9ec6759..9d10139b9 100644 --- a/src/core/monitor/CMakeLists.txt +++ b/src/core/monitor/CMakeLists.txt @@ -48,6 +48,7 @@ target_link_libraries(core_monitor Gnuradio::runtime protobuf::libprotobuf core_system_parameters + core_libs PRIVATE Boost::serialization ) @@ -59,7 +60,6 @@ target_include_directories(core_monitor ${GNSSSDR_SOURCE_DIR}/src/core/interfaces PRIVATE ${GNSSSDR_SOURCE_DIR}/src/algorithms/libs # for gnss_sdr_make_unique - ${GNSSSDR_SOURCE_DIR}/src/core/libs # for protobuf_cleanup_manager ) # Do not apply clang-tidy fixes to protobuf generated headers diff --git a/src/core/monitor/gnss_synchro_monitor.cc b/src/core/monitor/gnss_synchro_monitor.cc index 5f1aefce7..4668583f2 100644 --- a/src/core/monitor/gnss_synchro_monitor.cc +++ b/src/core/monitor/gnss_synchro_monitor.cc @@ -68,9 +68,6 @@ int gnss_synchro_monitor::general_work(int noutput_items __attribute__((unused)) { // Get the input buffer pointer const auto** in = reinterpret_cast(&input_items[0]); - // Convert to a vector and write to the UDP sink - std::vector stocks; - // Find maximum number of items across of all inputs int n_items_max = 0; for (int channel_index = 0; channel_index < d_nchannels; channel_index++) @@ -83,6 +80,8 @@ int gnss_synchro_monitor::general_work(int noutput_items __attribute__((unused)) { // Use the count variable to limit how many items are sent per channel count++; + // Gnss_Synchro objects selected in this round, sent in a single datagram + std::vector stocks; // Loop through each input stream channel for (int channel_index = 0; channel_index < d_nchannels; channel_index++) { @@ -101,13 +100,13 @@ int gnss_synchro_monitor::general_work(int noutput_items __attribute__((unused)) { // Reset count variable count = 0; + // Do not send empty datagrams + if (!stocks.empty()) + { + udp_sink_ptr->write_gnss_synchro(stocks); + } } } - // Do not send empty datagrams - if (!stocks.empty()) - { - udp_sink_ptr->write_gnss_synchro(stocks); - } // Not producing any outputs return 0; diff --git a/src/core/monitor/serdes_gnss_synchro.h b/src/core/monitor/serdes_gnss_synchro.h index a6e8e3eed..aefe71471 100644 --- a/src/core/monitor/serdes_gnss_synchro.h +++ b/src/core/monitor/serdes_gnss_synchro.h @@ -39,15 +39,9 @@ public: // compatible with the version of the headers we compiled against. GOOGLE_PROTOBUF_VERIFY_VERSION; - // This is a static instance of "protobuf resource cleaner" class, - // that would be destructed only once at the end of the program execution - // This prevents random crashes due to memory corruption - // And keeps memory leak checkers happy - volatile auto& protobuf_cleaner __attribute__((unused)) = protobuf_cleanup_manager::get(); - } - - ~Serdes_Gnss_Synchro() - { + // Make sure google::protobuf::ShutdownProtobufLibrary() is called only once, + // at the end of the program execution (see Protobuf_Cleanup_Manager) + Protobuf_Cleanup_Manager::get(); } inline Serdes_Gnss_Synchro(const Serdes_Gnss_Synchro& other) noexcept //!< Copy constructor diff --git a/tests/unit-tests/control-plane/gnss_synchro_monitor_test.cc b/tests/unit-tests/control-plane/gnss_synchro_monitor_test.cc index 6b7d244b4..5725258e8 100644 --- a/tests/unit-tests/control-plane/gnss_synchro_monitor_test.cc +++ b/tests/unit-tests/control-plane/gnss_synchro_monitor_test.cc @@ -8,7 +8,7 @@ * GNSS-SDR is a Global Navigation Satellite System software-defined receiver. * This file is part of GNSS-SDR. * - * Copyright (C) 2010-2020 (see AUTHORS file for a list of contributors) + * Copyright (C) 2010-2026 (see AUTHORS file for a list of contributors) * SPDX-License-Identifier: GPL-3.0-or-later * * ----------------------------------------------------------------------------- @@ -19,11 +19,15 @@ #include "gnss_synchro_monitor.h" #include #include -#include #include #include #include +#include +#include +#include +#include #include +#include #include #include @@ -36,13 +40,12 @@ class gnss_synchro_emitter : public gr::sync_block public: virtual ~gnss_synchro_emitter() = default; - // Emit syncros one by one with small pause to match actual "Monitor.enable_monitor=true" GNSS-SDR configuration behavior + // Emit synchros one by one with a small pause to match the actual "Monitor.enable_monitor=true" GNSS-SDR configuration behavior int work(int /*noutput_items*/, gr_vector_const_void_star& /*input_items*/, gr_vector_void_star& output_items) { auto** out = reinterpret_cast(&output_items[0]); if (counter >= d_nitems) { - d_finished = true; return WORK_DONE; } for (int i = 0; i < d_nchannels; i++) @@ -51,20 +54,10 @@ public: vgs[i].interp_TOW_ms++; } counter++; - usleep(d_interval_us); + std::this_thread::sleep_for(std::chrono::microseconds(d_interval_us)); return 1; } - void reset() - { - d_finished = false; - } - - bool finished() - { - return d_finished; - } - private: friend gnss_synchro_emitter_sptr gnss_synchro_make_emitter(int n_channels, int nitems, int interval_us); @@ -91,7 +84,6 @@ private: const int d_interval_us; std::vector vgs; int counter{0}; - bool d_finished{false}; }; gnss_synchro_emitter_sptr gnss_synchro_make_emitter(int n_channels, int nitems, int interval_us) @@ -102,15 +94,17 @@ gnss_synchro_emitter_sptr gnss_synchro_make_emitter(int n_channels, int nitems, TEST(Monitor, decimation) { - // Minimum number of channels to confirm, that decimation by 2 is not performed just by dropping all synchros from one channel + // Minimum number of channels to confirm that decimation by 2 is not performed just by dropping all synchros from one channel constexpr uint32_t NCHANNELS = 2; constexpr uint32_t NSYNCHRO = 16; constexpr uint32_t NDEC_MAX = 8; - // Limit receive the test execution time - constexpr uint32_t RX_LOOP_MAX_ITER = 32; + // Limit the test execution time in case something goes wrong + constexpr auto RX_TIMEOUT = std::chrono::seconds(5); constexpr uint32_t GNSS_SYNCHRO_INTERVAL_US = 1000; b_io_context io_context; - boost::asio::ip::udp::socket socket(io_context, boost::asio::ip::udp::endpoint(boost::asio::ip::udp::v4(), 1112)); + // Bind to an ephemeral port, so the test does not collide with a running receiver + boost::asio::ip::udp::socket socket(io_context, boost::asio::ip::udp::endpoint(boost::asio::ip::address_v4::loopback(), 0)); + const std::string udp_port = std::to_string(socket.local_endpoint().port()); std::vector recv_buf(8192); boost::asio::ip::udp::endpoint remote_endpoint; for (uint32_t ndec = 1; ndec <= NDEC_MAX; ndec <<= 1) @@ -133,25 +127,26 @@ TEST(Monitor, decimation) Serdes_Gnss_Synchro serdes = Serdes_Gnss_Synchro(); auto top_block = gr::make_top_block("monitor_test"); auto emitter = gnss_synchro_make_emitter(NCHANNELS, NSYNCHRO, GNSS_SYNCHRO_INTERVAL_US); - auto monitor = gnss_synchro_make_monitor(NCHANNELS, ndec, {"1112"}, {"127.0.0.1"}, true); + auto monitor = gnss_synchro_make_monitor(NCHANNELS, ndec, {udp_port}, {"127.0.0.1"}, true); for (uint32_t i = 0; i < NCHANNELS; i++) { top_block->connect(emitter, i, monitor, i); } top_block->start(); - for (uint32_t rx_iter = 0; rx_iter < RX_LOOP_MAX_ITER; rx_iter++) + const auto deadline = std::chrono::steady_clock::now() + RX_TIMEOUT; + while (std::chrono::steady_clock::now() < deadline) { - usleep(GNSS_SYNCHRO_INTERVAL_US); + std::this_thread::sleep_for(std::chrono::microseconds(GNSS_SYNCHRO_INTERVAL_US)); if (socket.available() == 0) { continue; } - size_t nbytes = socket.receive_from(boost::asio::buffer(recv_buf), remote_endpoint); - EXPECT_GT(nbytes, 0); - obs.ParseFromArray(recv_buf.data(), nbytes); + const std::size_t nbytes = socket.receive_from(boost::asio::buffer(recv_buf), remote_endpoint); + EXPECT_GT(nbytes, 0U); + EXPECT_TRUE(obs.ParseFromArray(recv_buf.data(), static_cast(nbytes))); const std::vector vgs_read = serdes.readProtobuffer(obs); - // validate deserialization result and add it's unique index to set - for (auto& gs : vgs_read) + // validate deserialization result and add its unique index to the set + for (const auto& gs : vgs_read) { EXPECT_EQ(gs.PRN, gs.Channel_ID + 1); received[gs.Channel_ID].insert(gs.interp_TOW_ms); @@ -164,8 +159,8 @@ TEST(Monitor, decimation) } top_block->stop(); top_block->wait(); - // Check the number of unique syncros per channel against the expected number - for (auto& gs_set : received) + // Check the number of unique synchros per channel against the expected number + for (const auto& gs_set : received) { EXPECT_EQ(gs_set.size(), nexpected); EXPECT_EQ(gs_set, expected);