From 2926904cf726cd37ddec9ffff8427254d5b73b5a Mon Sep 17 00:00:00 2001 From: Dhanya Thattil Date: Tue, 8 Jul 2025 17:25:23 +0200 Subject: [PATCH] cleaning up properly , semaphore leaks, child process/thread throwing handled --- .../src/CommandLineOptions.cpp | 7 +-- .../src/FrameSynchronizerApp.cpp | 47 ++++++++++++------- slsReceiverSoftware/src/MultiReceiverApp.cpp | 12 ++--- slsReceiverSoftware/src/Receiver.cpp | 4 +- slsReceiverSoftware/src/ReceiverApp.cpp | 4 +- slsReceiverSoftware/tests/test-Apps.cpp | 18 +++---- 6 files changed, 52 insertions(+), 40 deletions(-) diff --git a/slsReceiverSoftware/src/CommandLineOptions.cpp b/slsReceiverSoftware/src/CommandLineOptions.cpp index bed4a7a0f..392b6bb34 100644 --- a/slsReceiverSoftware/src/CommandLineOptions.cpp +++ b/slsReceiverSoftware/src/CommandLineOptions.cpp @@ -28,7 +28,8 @@ ParsedOptions CommandLineOptions::parse(int argc, char *argv[]) { MultiReceiverOptions multi; FrameSyncOptions frame; - auto optString = buildOptString(); + // leading '+' to stop parsing at first non-option argument (to allow for deprecated options) + auto optString = "+" + buildOptString(); auto longOptions = buildOptionList(); optind = 0; // reset getopt @@ -52,7 +53,7 @@ ParsedOptions CommandLineOptions::parse(int argc, char *argv[]) { handleAppSpecificOption(opt, optarg, base, multi, frame); break; default: - throw sls::RuntimeError("Invalid arguments." + getHelpMessage()); + throw sls::RuntimeError("Invalidddd arguments." + getHelpMessage()); } } @@ -237,7 +238,7 @@ void CommandLineOptions::handleAppSpecificOption(int opt, const char *optarg, } case 't': - LOG(sls::logWARNING) << "Deprecated option 't' and '--rx_tcport'. Use " + LOG(sls::logWARNING) << "Deprecated option '-t' and '--rx_tcport'. Use " "'p' or '--port' instead."; base.port = parsePort(optarg); break; diff --git a/slsReceiverSoftware/src/FrameSynchronizerApp.cpp b/slsReceiverSoftware/src/FrameSynchronizerApp.cpp index b164601ad..ca815f283 100644 --- a/slsReceiverSoftware/src/FrameSynchronizerApp.cpp +++ b/slsReceiverSoftware/src/FrameSynchronizerApp.cpp @@ -503,16 +503,15 @@ void sigInterruptHandler(int p) { } int main(int argc, char *argv[]) { - CommandLineOptions cli(AppType::SingleReceiver); + CommandLineOptions cli(AppType::FrameSynchronizer); ParsedOptions opts; try { opts = cli.parse(argc, argv); } catch (sls::RuntimeError &e) { return EXIT_FAILURE; } - auto &o = std::get(opts); auto &f = std::get(opts); - if (o.versionRequested || o.helpRequested) { + if (f.versionRequested || f.helpRequested) { return EXIT_SUCCESS; } @@ -525,7 +524,7 @@ int main(int argc, char *argv[]) { semaphores.resize(f.numReceivers); for (auto &s : semaphores) { - sem_init(&s, 1, 0); + sem_init(&s, 0, 0); } FrameStatus stat{true, false, f.numReceivers}; @@ -537,24 +536,35 @@ int main(int argc, char *argv[]) { std::thread combinerThread(Correlate, &stat); for (int i = 0; i != f.numReceivers; ++i) { - uint16_t port = o.port + i; + uint16_t port = f.port + i; sem_t *semaphore = &semaphores[i]; threads.emplace_back([i, semaphore, port, user_data]() { - sls::Receiver receiver(port); - receiver.registerCallBackStartAcquisition(StartAcquisitionCallback, - user_data); - receiver.registerCallBackAcquisitionFinished( - AcquisitionFinishedCallback, user_data); - receiver.registerCallBackRawDataReady(GetDataCallback, user_data); + LOG(sls::logINFOBLUE) + << "Thread " << i << " [ Tid: " << gettid() << ']'; + try { + sls::Receiver receiver(port); + receiver.registerCallBackStartAcquisition(StartAcquisitionCallback, + user_data); + receiver.registerCallBackAcquisitionFinished( + AcquisitionFinishedCallback, user_data); + receiver.registerCallBackRawDataReady(GetDataCallback, user_data); - /** - as long as no Ctrl+C */ - // each child shares the common semaphore - sem_wait(semaphore); - sem_destroy(semaphore); - - // clean up frames - if (i == 0) + /** - as long as no Ctrl+C */ + // each child shares the common semaphore + sem_wait(semaphore); + } catch (...) { + LOG(sls::logINFOBLUE) + << "Exiting Thread " << i << " [ Tid: " << gettid() << " ]"; + for (auto &s : semaphores) + sem_destroy(&s); cleanup(); + if (global_frame_status) + sem_destroy(&(global_frame_status->available)); + std::exit(EXIT_FAILURE); + } + LOG(sls::logINFOBLUE) + << "Exiting Thread " << i << " [ Tid: " << gettid() << " ]"; + sem_destroy(semaphore); }); } @@ -562,6 +572,7 @@ int main(int argc, char *argv[]) { t.join(); } + cleanup(); { std::lock_guard lock(stat.mtx); stat.terminate = true; diff --git a/slsReceiverSoftware/src/MultiReceiverApp.cpp b/slsReceiverSoftware/src/MultiReceiverApp.cpp index 00a032697..75b8e2a9a 100644 --- a/slsReceiverSoftware/src/MultiReceiverApp.cpp +++ b/slsReceiverSoftware/src/MultiReceiverApp.cpp @@ -143,16 +143,15 @@ void sigInterruptHandler(int signal) { int main(int argc, char *argv[]) { - CommandLineOptions cli(AppType::SingleReceiver); + CommandLineOptions cli(AppType::MultiReceiver); ParsedOptions opts; try { opts = cli.parse(argc, argv); } catch (sls::RuntimeError &e) { return EXIT_FAILURE; } - auto& o = std::get(opts); auto &m = std::get(opts); - if (o.versionRequested || o.helpRequested) { + if (m.versionRequested || m.helpRequested) { return EXIT_SUCCESS; } @@ -185,7 +184,7 @@ int main(int argc, char *argv[]) { << "Child process " << i << " [ Tid: " << gettid() << ']'; try { - uint16_t port = o.port + i; + uint16_t port = m.port + i; sls::Receiver receiver(port); /** - register callbacks. remember to set file write enable @@ -215,6 +214,7 @@ int main(int argc, char *argv[]) { sem_destroy(&semaphore); } catch (...) { + sem_destroy(&semaphore); LOG(sls::logINFOBLUE) << "Exiting Child Process [ Tid: " << gettid() << " ]"; throw; @@ -250,10 +250,6 @@ int main(int argc, char *argv[]) { break; } } - - // child closed - LOG(sls::logINFOBLUE) - << "Exiting Child Process [ Tid: " << childPid << ']'; } std::cout << "Goodbye!\n"; diff --git a/slsReceiverSoftware/src/Receiver.cpp b/slsReceiverSoftware/src/Receiver.cpp index 3b1890965..43906dbe9 100644 --- a/slsReceiverSoftware/src/Receiver.cpp +++ b/slsReceiverSoftware/src/Receiver.cpp @@ -29,8 +29,10 @@ Receiver::~Receiver() = default; Receiver::Receiver(uint16_t port) { validatePortNumber(port); +//#ifdef SLS_USE_TESTS + if (port == 9999) throw RuntimeError("throwing for 9999 test"); +//#endif tcpipInterface = make_unique(port); - if (port == 1957) throw RuntimeError("throwing for 1957"); } std::string Receiver::getReceiverVersion() { diff --git a/slsReceiverSoftware/src/ReceiverApp.cpp b/slsReceiverSoftware/src/ReceiverApp.cpp index 52a19c8cf..917c3b6d6 100644 --- a/slsReceiverSoftware/src/ReceiverApp.cpp +++ b/slsReceiverSoftware/src/ReceiverApp.cpp @@ -58,7 +58,9 @@ int main(int argc, char *argv[]) { sem_wait(&semaphore); sem_destroy(&semaphore); } catch (...) { - // pass + sem_destroy(&semaphore); + LOG(sls::logINFOBLUE) << "Exiting [ Tid: " << gettid() << " ]"; + throw; } LOG(sls::logINFOBLUE) << "Exiting [ Tid: " << gettid() << " ]"; LOG(sls::logINFO) << "Exiting Receiver"; diff --git a/slsReceiverSoftware/tests/test-Apps.cpp b/slsReceiverSoftware/tests/test-Apps.cpp index b85f3f786..9b323e6dd 100644 --- a/slsReceiverSoftware/tests/test-Apps.cpp +++ b/slsReceiverSoftware/tests/test-Apps.cpp @@ -200,7 +200,7 @@ namespace sls { REQUIRE_THROWS(s.parse({"", "110", "1954"}));// mix order REQUIRE_THROWS(s.parse({"", "1023", "10"}));// privileged port REQUIRE_THROWS(s.parse({"", "2000", "0"}));// invalid num receivers - REQUIRE_THROWS(s.parse({"", "2000", "1000"}));// invalid num receivers + REQUIRE_THROWS(s.parse({"", "2000", "1001"}));// invalid num receivers REQUIRE_THROWS(s.parse({"", "1954", "1", "2"}));// invalid 3rd opt REQUIRE_NOTHROW(s.parse({""})); @@ -217,13 +217,13 @@ namespace sls { REQUIRE(o.callbackEnabled == false); } else if constexpr (is_type(o)) { - REQUIRE(o.port == 1958); - REQUIRE(o.numReceivers == 10); + REQUIRE(o.port == 1954); + REQUIRE(o.numReceivers == 1); REQUIRE(o.printHeaders == false); // default } }, opts); - auto opts = s.parse({"", "1958", "10"}); + opts = s.parse({"", "1958", "10"}); std::visit([&](const auto& o) { if constexpr (is_type(o)) { REQUIRE(o.port == 1958); @@ -237,7 +237,7 @@ namespace sls { } }, opts); - auto opts = s.parse({"", "1958", "1", "1"}); + opts = s.parse({"", "1958", "10", "1"}); std::visit([&](const auto& o) { if constexpr (is_type(o)) { REQUIRE(o.port == 1958); @@ -264,13 +264,13 @@ namespace sls { REQUIRE(n == 1); REQUIRE(o == false); - auto [p, n, o] = CommandLineOptions::ParseDeprecated({"", "1955", "6"}); - REQUIRE(p == 1954); + std::tie(p, n, o) = CommandLineOptions::ParseDeprecated({"", "1955", "6"}); + REQUIRE(p == 1955); REQUIRE(n == 6); REQUIRE(o == false); - auto [p, n, o] = CommandLineOptions::ParseDeprecated({"", "1955", "6", "1"}); - REQUIRE(p == 1954); + std::tie(p, n, o) = CommandLineOptions::ParseDeprecated({"", "1955", "6", "1"}); + REQUIRE(p == 1955); REQUIRE(n == 6); REQUIRE(o == true); }