From 5571a71e4e28d00a6ade3a0dc86126b347e6c163 Mon Sep 17 00:00:00 2001 From: Bryan Biedenkapp Date: Sun, 18 Feb 2024 09:19:10 -0500 Subject: [PATCH] replace manual lock/unlock with lock_guard to ensure a lock is held in a scope, and released when a scope is closed; --- src/common/lookups/IdenTableLookup.cpp | 113 +++++---- src/common/lookups/IdenTableLookup.h | 8 +- src/common/lookups/LookupTable.h | 29 +-- src/common/lookups/RadioIdLookup.cpp | 188 +++++++-------- src/common/lookups/RadioIdLookup.h | 8 +- src/common/lookups/TalkgroupRulesLookup.cpp | 251 +++++++++----------- src/common/lookups/TalkgroupRulesLookup.h | 2 +- src/common/network/RawFrameQueue.cpp | 42 ++-- src/common/network/RawFrameQueue.h | 2 +- src/fne/network/FNENetwork.cpp | 41 ++-- src/fne/network/FNENetwork.h | 2 +- src/host/Host.cpp | 30 ++- 12 files changed, 357 insertions(+), 359 deletions(-) diff --git a/src/common/lookups/IdenTableLookup.cpp b/src/common/lookups/IdenTableLookup.cpp index 3b3fdb16..6dd0fcf8 100644 --- a/src/common/lookups/IdenTableLookup.cpp +++ b/src/common/lookups/IdenTableLookup.cpp @@ -7,7 +7,7 @@ * @package DVM / Common Library * @license GPLv2 License (https://opensource.org/licenses/GPL-2.0) * -* Copyright (C) 2018-2022 Bryan Biedenkapp, N2PLL +* Copyright (C) 2018-2022,2024 Bryan Biedenkapp, N2PLL * Copyright (c) 2024 Patrick McDonnell, W3AXL * */ @@ -21,6 +21,12 @@ using namespace lookups; #include #include +// --------------------------------------------------------------------------- +// Static Class Members +// --------------------------------------------------------------------------- + +std::mutex IdenTableLookup::m_mutex; + // --------------------------------------------------------------------------- // Public Class Members // --------------------------------------------------------------------------- @@ -35,6 +41,15 @@ IdenTableLookup::IdenTableLookup(const std::string& filename, uint32_t reloadTim /* stub */ } +/// +/// Clears all entries from the lookup table. +/// +void IdenTableLookup::clear() +{ + std::lock_guard lock(m_mutex); + m_table.clear(); +} + /// /// Finds a table entry in this lookup table. /// @@ -44,15 +59,12 @@ IdenTable IdenTableLookup::find(uint32_t id) { IdenTable entry; - m_mutex.lock(); - { - try { - entry = m_table.at(id); - } catch (...) { - /* stub */ - } + std::lock_guard lock(m_mutex); + try { + entry = m_table.at(id); + } catch (...) { + /* stub */ } - m_mutex.unlock(); float chBandwidthKhz = entry.chBandwidthKhz(); if (chBandwidthKhz == 0.0F) @@ -105,57 +117,55 @@ bool IdenTableLookup::load() // clear table clear(); - m_mutex.lock(); - { - // read lines from file - std::string line; - while (std::getline(file, line)) { - if (line.length() > 0) { - if (line.at(0) == '#') - continue; - - // tokenize line - std::string next; - std::vector parsed; - char delim = ','; - - for (auto it = line.begin(); it != line.end(); it++) { - if (*it == delim) { - if (!next.empty()) { - parsed.push_back(next); - next.clear(); - } + std::lock_guard lock(m_mutex); + + // read lines from file + std::string line; + while (std::getline(file, line)) { + if (line.length() > 0) { + if (line.at(0) == '#') + continue; + + // tokenize line + std::string next; + std::vector parsed; + char delim = ','; + + for (auto it = line.begin(); it != line.end(); it++) { + if (*it == delim) { + if (!next.empty()) { + parsed.push_back(next); + next.clear(); } - else - next += *it; } - if (!next.empty()) - parsed.push_back(next); + else + next += *it; + } + if (!next.empty()) + parsed.push_back(next); - // parse tokenized line - uint8_t channelId = (uint8_t)::atoi(parsed[0].c_str()); - uint32_t baseFrequency = (uint32_t)::atoi(parsed[1].c_str()); - float chSpaceKhz = float(::atof(parsed[2].c_str())); - float txOffsetMhz = float(::atof(parsed[3].c_str())); - float chBandwidthKhz = float(::atof(parsed[4].c_str())); + // parse tokenized line + uint8_t channelId = (uint8_t)::atoi(parsed[0].c_str()); + uint32_t baseFrequency = (uint32_t)::atoi(parsed[1].c_str()); + float chSpaceKhz = float(::atof(parsed[2].c_str())); + float txOffsetMhz = float(::atof(parsed[3].c_str())); + float chBandwidthKhz = float(::atof(parsed[4].c_str())); - if (chSpaceKhz == 0.0F) - chSpaceKhz = chBandwidthKhz / 2; - if (chSpaceKhz < 0.125F) // clamp to 125 Hz - chSpaceKhz = 0.125F; - if (chSpaceKhz > 125000.0F) // clamp to 125 kHz - chSpaceKhz = 125000.0F; + if (chSpaceKhz == 0.0F) + chSpaceKhz = chBandwidthKhz / 2; + if (chSpaceKhz < 0.125F) // clamp to 125 Hz + chSpaceKhz = 0.125F; + if (chSpaceKhz > 125000.0F) // clamp to 125 kHz + chSpaceKhz = 125000.0F; - IdenTable entry = IdenTable(channelId, baseFrequency, chSpaceKhz, txOffsetMhz, chBandwidthKhz); + IdenTable entry = IdenTable(channelId, baseFrequency, chSpaceKhz, txOffsetMhz, chBandwidthKhz); - LogMessage(LOG_HOST, "Channel Id %u: BaseFrequency = %uHz, TXOffsetMhz = %fMHz, BandwidthKhz = %fKHz, SpaceKhz = %fKHz", - entry.channelId(), entry.baseFrequency(), entry.txOffsetMhz(), entry.chBandwidthKhz(), entry.chSpaceKhz()); + LogMessage(LOG_HOST, "Channel Id %u: BaseFrequency = %uHz, TXOffsetMhz = %fMHz, BandwidthKhz = %fKHz, SpaceKhz = %fKHz", + entry.channelId(), entry.baseFrequency(), entry.txOffsetMhz(), entry.chBandwidthKhz(), entry.chSpaceKhz()); - m_table[channelId] = entry; - } + m_table[channelId] = entry; } } - m_mutex.unlock(); file.close(); @@ -174,6 +184,5 @@ bool IdenTableLookup::load() /// True, if lookup table was saved, otherwise false. bool IdenTableLookup::save() { - /// TODO TODO TODO actually save stuff return false; } \ No newline at end of file diff --git a/src/common/lookups/IdenTableLookup.h b/src/common/lookups/IdenTableLookup.h index 6618b27f..ac7d2094 100644 --- a/src/common/lookups/IdenTableLookup.h +++ b/src/common/lookups/IdenTableLookup.h @@ -7,7 +7,7 @@ * @package DVM / Common Library * @license GPLv2 License (https://opensource.org/licenses/GPL-2.0) * -* Copyright (C) 2018-2022 Bryan Biedenkapp, N2PLL +* Copyright (C) 2018-2022,2024 Bryan Biedenkapp, N2PLL * Copyright (c) 2024 Patrick McDonnell, W3AXL * */ @@ -96,6 +96,9 @@ namespace lookups /// Initializes a new instance of the IdenTableLookup class. IdenTableLookup(const std::string& filename, uint32_t reloadTime); + /// Clears all entries from the lookup table. + void clear() override; + /// Finds a table entry in this lookup table. IdenTable find(uint32_t id) override; /// Returns the list of entries in this lookup table. @@ -109,6 +112,9 @@ namespace lookups /// Saves the table to the passed lookup table file. /// True, if lookup table was saved, otherwise false. bool save() override; + + private: + static std::mutex m_mutex; }; } // namespace lookups diff --git a/src/common/lookups/LookupTable.h b/src/common/lookups/LookupTable.h index 0b219483..2eef168f 100644 --- a/src/common/lookups/LookupTable.h +++ b/src/common/lookups/LookupTable.h @@ -7,7 +7,7 @@ * @package DVM / Common Library * @license GPLv2 License (https://opensource.org/licenses/GPL-2.0) * -* Copyright (C) 2018-2022 Bryan Biedenkapp, N2PLL +* Copyright (C) 2018-2022,2024 Bryan Biedenkapp, N2PLL * Copyright (c) 2024 Patrick McDonnell, W3AXL * */ @@ -98,11 +98,9 @@ namespace lookups /// Clears all entries from the lookup table. virtual void clear() { - m_mutex.lock(); - { - m_table.clear(); - } - m_mutex.unlock(); + // bryanb: this is not thread-safe and thread saftey should be implemented + // on the derived class + m_table.clear(); } /// Helper to check if this lookup table has the specified unique ID. @@ -110,19 +108,13 @@ namespace lookups /// True, if the lookup table has an entry by the specified unique ID, otherwise false. virtual bool hasEntry(uint32_t id) { - m_mutex.lock(); - { - try { - m_table.at(id); - m_mutex.unlock(); - return true; - } - catch (...) { - m_mutex.unlock(); - return false; - } + try { + m_table.at(id); + return true; + } + catch (...) { + return false; } - m_mutex.unlock(); return false; } @@ -140,7 +132,6 @@ namespace lookups std::string m_filename; uint32_t m_reloadTime; std::unordered_map m_table; - std::mutex m_mutex; bool m_stop; /// Loads the table from the passed lookup table file. diff --git a/src/common/lookups/RadioIdLookup.cpp b/src/common/lookups/RadioIdLookup.cpp index ba0fb379..85cab851 100644 --- a/src/common/lookups/RadioIdLookup.cpp +++ b/src/common/lookups/RadioIdLookup.cpp @@ -24,6 +24,12 @@ using namespace lookups; #include #include +// --------------------------------------------------------------------------- +// Static Class Members +// --------------------------------------------------------------------------- + +std::mutex RadioIdLookup::m_mutex; + // --------------------------------------------------------------------------- // Public Class Members // --------------------------------------------------------------------------- @@ -40,6 +46,15 @@ RadioIdLookup::RadioIdLookup(const std::string& filename, uint32_t reloadTime, b /* stub */ } +/// +/// Clears all entries from the lookup table. +/// +void RadioIdLookup::clear() +{ + std::lock_guard lock(m_mutex); + m_table.clear(); +} + /// /// Toggles the specified radio ID enabled or disabled. /// @@ -65,21 +80,18 @@ void RadioIdLookup::addEntry(uint32_t id, bool enabled, const std::string& alias RadioId entry = RadioId(enabled, false, alias); - m_mutex.lock(); - { - try { - RadioId _entry = m_table.at(id); + std::lock_guard lock(m_mutex); + try { + RadioId _entry = m_table.at(id); - // if the enabled value doesn't match -- override with the intended - if (_entry.radioEnabled() != enabled) { - _entry = RadioId(enabled, false, alias); - m_table[id] = _entry; - } - } catch (...) { - m_table[id] = entry; + // if the enabled value doesn't match -- override with the intended + if (_entry.radioEnabled() != enabled) { + _entry = RadioId(enabled, false, alias); + m_table[id] = _entry; } + } catch (...) { + m_table[id] = entry; } - m_mutex.unlock(); } /// @@ -88,16 +100,13 @@ void RadioIdLookup::addEntry(uint32_t id, bool enabled, const std::string& alias /// Unique ID to erase. void RadioIdLookup::eraseEntry(uint32_t id) { - m_mutex.lock(); - { - try { - m_table.at(id); - m_table.erase(id); - } catch (...) { - /* stub */ - } + std::lock_guard lock(m_mutex); + try { + m_table.at(id); + m_table.erase(id); + } catch (...) { + /* stub */ } - m_mutex.unlock(); } /// @@ -113,15 +122,12 @@ RadioId RadioIdLookup::find(uint32_t id) return RadioId(true, false); } - m_mutex.lock(); - { - try { - entry = m_table.at(id); - } catch (...) { - entry = RadioId(false, true); - } + std::lock_guard lock(m_mutex); + try { + entry = m_table.at(id); + } catch (...) { + entry = RadioId(false, true); } - m_mutex.unlock(); return entry; } @@ -166,51 +172,49 @@ bool RadioIdLookup::load() // clear table clear(); - m_mutex.lock(); - { - // read lines from file - std::string line; - while (std::getline(file, line)) { - if (line.length() > 0) { - // Skip comments with # - if (line.at(0) == '#') - continue; - - // tokenize line - std::string next; - std::vector parsed; - char delim = ','; - - for (char c : line) { - if (c == delim) { - if (!next.empty()) { - parsed.push_back(next); - next.clear(); - } + std::lock_guard lock(m_mutex); + + // read lines from file + std::string line; + while (std::getline(file, line)) { + if (line.length() > 0) { + // Skip comments with # + if (line.at(0) == '#') + continue; + + // tokenize line + std::string next; + std::vector parsed; + char delim = ','; + + for (char c : line) { + if (c == delim) { + if (!next.empty()) { + parsed.push_back(next); + next.clear(); } - else - next += c; - } - if (!next.empty()) - parsed.push_back(next); - - // parse tokenized line - uint32_t id = ::atoi(parsed[0].c_str()); - bool radioEnabled = ::atoi(parsed[1].c_str()) == 1; - bool radioDefault = false; - - // Check for an optional alias field - if (parsed.size() >= 3) { - m_table[id] = RadioId(radioEnabled, radioDefault, parsed[2]); - LogDebug(LOG_HOST, "Loaded RID %u (%s) into RID lookup table", id, parsed[2].c_str()); - } else { - m_table[id] = RadioId(radioEnabled, radioDefault); - LogDebug(LOG_HOST, "Loaded RID %u into RID lookup table", id); } + else + next += c; + } + if (!next.empty()) + parsed.push_back(next); + + // parse tokenized line + uint32_t id = ::atoi(parsed[0].c_str()); + bool radioEnabled = ::atoi(parsed[1].c_str()) == 1; + bool radioDefault = false; + + // Check for an optional alias field + if (parsed.size() >= 3) { + m_table[id] = RadioId(radioEnabled, radioDefault, parsed[2]); + LogDebug(LOG_HOST, "Loaded RID %u (%s) into RID lookup table", id, parsed[2].c_str()); + } else { + m_table[id] = RadioId(radioEnabled, radioDefault); + LogDebug(LOG_HOST, "Loaded RID %u into RID lookup table", id); } } } - m_mutex.unlock(); file.close(); @@ -244,32 +248,30 @@ bool RadioIdLookup::save() // Counter for lines written unsigned int lines = 0; - m_mutex.lock(); - { - // String for writing - std::string line; - // iterate over each entry in the RID lookup and write it to the open file - for (auto& entry: m_table) { - // Get the parameters - uint32_t rid = entry.first; - bool enabled = entry.second.radioEnabled(); - std::string alias = entry.second.radioAlias(); - // Format into a string - line = std::to_string(rid) + "," + std::to_string(enabled) + ","; - // Add the alias if we have one - if (alias.length() > 0) { - line += alias; - line += ","; - } - // Add the newline - line += "\n"; - // Write to file - file << line; - // Increment - lines++; + std::lock_guard lock(m_mutex); + + // String for writing + std::string line; + // iterate over each entry in the RID lookup and write it to the open file + for (auto& entry: m_table) { + // Get the parameters + uint32_t rid = entry.first; + bool enabled = entry.second.radioEnabled(); + std::string alias = entry.second.radioAlias(); + // Format into a string + line = std::to_string(rid) + "," + std::to_string(enabled) + ","; + // Add the alias if we have one + if (alias.length() > 0) { + line += alias; + line += ","; } + // Add the newline + line += "\n"; + // Write to file + file << line; + // Increment + lines++; } - m_mutex.unlock(); file.close(); diff --git a/src/common/lookups/RadioIdLookup.h b/src/common/lookups/RadioIdLookup.h index a357cbe1..3c207962 100644 --- a/src/common/lookups/RadioIdLookup.h +++ b/src/common/lookups/RadioIdLookup.h @@ -9,7 +9,7 @@ * @license GPLv2 License (https://opensource.org/licenses/GPL-2.0) * * Copyright (C) 2016 Jonathan Naylor, G4KLX -* Copyright (C) 2017-2022 Bryan Biedenkapp, N2PLL +* Copyright (C) 2017-2022,2024 Bryan Biedenkapp, N2PLL * Copyright (c) 2024 Patrick McDonnell, W3AXL * */ @@ -114,6 +114,9 @@ namespace lookups /// Initializes a new instance of the RadioIdLookup class. RadioIdLookup(const std::string& filename, uint32_t reloadTime, bool ridAcl); + /// Clears all entries from the lookup table. + void clear() override; + /// Toggles the specified radio ID enabled or disabled. void toggleEntry(uint32_t id, bool enabled); @@ -141,6 +144,9 @@ namespace lookups /// Saves the table to the passed lookup table file. /// True, if lookup table was saved, otherwise false. bool save() override; + + private: + static std::mutex m_mutex; }; } // namespace lookups diff --git a/src/common/lookups/TalkgroupRulesLookup.cpp b/src/common/lookups/TalkgroupRulesLookup.cpp index 203421fb..7864912c 100644 --- a/src/common/lookups/TalkgroupRulesLookup.cpp +++ b/src/common/lookups/TalkgroupRulesLookup.cpp @@ -21,6 +21,12 @@ using namespace lookups; #include #include +// --------------------------------------------------------------------------- +// Static Class Members +// --------------------------------------------------------------------------- + +std::mutex TalkgroupRulesLookup::m_mutex; + // --------------------------------------------------------------------------- // Public Class Members // --------------------------------------------------------------------------- @@ -105,11 +111,8 @@ bool TalkgroupRulesLookup::read() /// void TalkgroupRulesLookup::clear() { - m_mutex.lock(); - { - m_groupVoice.clear(); - } - m_mutex.unlock(); + std::lock_guard lock(m_mutex); + m_groupVoice.clear(); } /// @@ -126,40 +129,37 @@ void TalkgroupRulesLookup::addEntry(uint32_t id, uint8_t slot, bool enabled) source.tgSlot(slot); config.active(enabled); - m_mutex.lock(); - { - auto it = std::find_if(m_groupVoice.begin(), m_groupVoice.end(), - [&](TalkgroupRuleGroupVoice x) - { - if (slot != 0U) { - return x.source().tgId() == id && x.source().tgSlot() == slot; - } - - return x.source().tgId() == id; - }); - if (it != m_groupVoice.end()) { - source = it->source(); - source.tgId(id); - source.tgSlot(slot); - - config = it->config(); - config.active(enabled); - - TalkgroupRuleGroupVoice entry = *it; - entry.config(config); - entry.source(source); - - m_groupVoice[it - m_groupVoice.begin()] = entry; - } - else { - TalkgroupRuleGroupVoice entry; - entry.config(config); - entry.source(source); + std::lock_guard lock(m_mutex); + auto it = std::find_if(m_groupVoice.begin(), m_groupVoice.end(), + [&](TalkgroupRuleGroupVoice x) + { + if (slot != 0U) { + return x.source().tgId() == id && x.source().tgSlot() == slot; + } - m_groupVoice.push_back(entry); - } + return x.source().tgId() == id; + }); + if (it != m_groupVoice.end()) { + source = it->source(); + source.tgId(id); + source.tgSlot(slot); + + config = it->config(); + config.active(enabled); + + TalkgroupRuleGroupVoice entry = *it; + entry.config(config); + entry.source(source); + + m_groupVoice[it - m_groupVoice.begin()] = entry; + } + else { + TalkgroupRuleGroupVoice entry; + entry.config(config); + entry.source(source); + + m_groupVoice.push_back(entry); } - m_mutex.unlock(); } /// @@ -175,25 +175,22 @@ void TalkgroupRulesLookup::addEntry(TalkgroupRuleGroupVoice groupVoice) uint32_t id = entry.source().tgId(); uint8_t slot = entry.source().tgSlot(); - m_mutex.lock(); - { - auto it = std::find_if(m_groupVoice.begin(), m_groupVoice.end(), - [&](TalkgroupRuleGroupVoice x) - { - if (slot != 0U) { - return x.source().tgId() == id && x.source().tgSlot() == slot; - } - - return x.source().tgId() == id; - }); - if (it != m_groupVoice.end()) { - m_groupVoice[it - m_groupVoice.begin()] = entry; - } - else { - m_groupVoice.push_back(entry); - } + std::lock_guard lock(m_mutex); + auto it = std::find_if(m_groupVoice.begin(), m_groupVoice.end(), + [&](TalkgroupRuleGroupVoice x) + { + if (slot != 0U) { + return x.source().tgId() == id && x.source().tgSlot() == slot; + } + + return x.source().tgId() == id; + }); + if (it != m_groupVoice.end()) { + m_groupVoice[it - m_groupVoice.begin()] = entry; + } + else { + m_groupVoice.push_back(entry); } - m_mutex.unlock(); } /// @@ -203,14 +200,11 @@ void TalkgroupRulesLookup::addEntry(TalkgroupRuleGroupVoice groupVoice) /// DMR slot this talkgroup is valid on. void TalkgroupRulesLookup::eraseEntry(uint32_t id, uint8_t slot) { - m_mutex.lock(); - { - auto it = std::find_if(m_groupVoice.begin(), m_groupVoice.end(), [&](TalkgroupRuleGroupVoice x) { return x.source().tgId() == id && x.source().tgSlot() == slot; }); - if (it != m_groupVoice.end()) { - m_groupVoice.erase(it); - } + std::lock_guard lock(m_mutex); + auto it = std::find_if(m_groupVoice.begin(), m_groupVoice.end(), [&](TalkgroupRuleGroupVoice x) { return x.source().tgId() == id && x.source().tgSlot() == slot; }); + if (it != m_groupVoice.end()) { + m_groupVoice.erase(it); } - m_mutex.unlock(); } /// @@ -223,24 +217,21 @@ TalkgroupRuleGroupVoice TalkgroupRulesLookup::find(uint32_t id, uint8_t slot) { TalkgroupRuleGroupVoice entry; - m_mutex.lock(); - { - auto it = std::find_if(m_groupVoice.begin(), m_groupVoice.end(), - [&](TalkgroupRuleGroupVoice x) - { - if (slot != 0U) { - return x.source().tgId() == id && x.source().tgSlot() == slot; - } - - return x.source().tgId() == id; - }); - if (it != m_groupVoice.end()) { - entry = *it; - } else { - entry = TalkgroupRuleGroupVoice(); - } + std::lock_guard lock(m_mutex); + auto it = std::find_if(m_groupVoice.begin(), m_groupVoice.end(), + [&](TalkgroupRuleGroupVoice x) + { + if (slot != 0U) { + return x.source().tgId() == id && x.source().tgSlot() == slot; + } + + return x.source().tgId() == id; + }); + if (it != m_groupVoice.end()) { + entry = *it; + } else { + entry = TalkgroupRuleGroupVoice(); } - m_mutex.unlock(); return entry; } @@ -256,36 +247,33 @@ TalkgroupRuleGroupVoice TalkgroupRulesLookup::findByRewrite(uint32_t peerId, uin { TalkgroupRuleGroupVoice entry; - m_mutex.lock(); - { - auto it = std::find_if(m_groupVoice.begin(), m_groupVoice.end(), - [&](TalkgroupRuleGroupVoice x) - { - if (x.config().rewrite().size() == 0) - return false; - - std::vector rewrite = x.config().rewrite(); - auto innerIt = std::find_if(rewrite.begin(), rewrite.end(), - [&](TalkgroupRuleRewrite y) - { - if (slot != 0U) { - return y.peerId() == peerId && y.tgId() == id && y.tgSlot() == slot; - } - - return y.peerId() == peerId && y.tgId() == id; - }); - - if (innerIt != rewrite.end()) - return true; + std::lock_guard lock(m_mutex); + auto it = std::find_if(m_groupVoice.begin(), m_groupVoice.end(), + [&](TalkgroupRuleGroupVoice x) + { + if (x.config().rewrite().size() == 0) return false; - }); - if (it != m_groupVoice.end()) { - entry = *it; - } else { - entry = TalkgroupRuleGroupVoice(); - } + + std::vector rewrite = x.config().rewrite(); + auto innerIt = std::find_if(rewrite.begin(), rewrite.end(), + [&](TalkgroupRuleRewrite y) + { + if (slot != 0U) { + return y.peerId() == peerId && y.tgId() == id && y.tgSlot() == slot; + } + + return y.peerId() == peerId && y.tgId() == id; + }); + + if (innerIt != rewrite.end()) + return true; + return false; + }); + if (it != m_groupVoice.end()) { + entry = *it; + } else { + entry = TalkgroupRuleGroupVoice(); } - m_mutex.unlock(); return entry; } @@ -336,37 +324,34 @@ bool TalkgroupRulesLookup::load() // clear table clear(); - m_mutex.lock(); - { - yaml::Node& groupVoiceList = m_rules["groupVoice"]; + std::lock_guard lock(m_mutex); + yaml::Node& groupVoiceList = m_rules["groupVoice"]; - if (groupVoiceList.size() == 0U) { - ::LogError(LOG_HOST, "No group voice rules list defined!"); - return false; - } - - for (size_t i = 0; i < groupVoiceList.size(); i++) { - TalkgroupRuleGroupVoice groupVoice = TalkgroupRuleGroupVoice(groupVoiceList[i]); - m_groupVoice.push_back(groupVoice); + if (groupVoiceList.size() == 0U) { + ::LogError(LOG_HOST, "No group voice rules list defined!"); + return false; + } - std::string groupName = groupVoice.name(); - uint32_t tgId = groupVoice.source().tgId(); - uint8_t tgSlot = groupVoice.source().tgSlot(); - bool active = groupVoice.config().active(); - bool parrot = groupVoice.config().parrot(); + for (size_t i = 0; i < groupVoiceList.size(); i++) { + TalkgroupRuleGroupVoice groupVoice = TalkgroupRuleGroupVoice(groupVoiceList[i]); + m_groupVoice.push_back(groupVoice); - uint32_t incCount = groupVoice.config().inclusion().size(); - uint32_t excCount = groupVoice.config().exclusion().size(); - uint32_t rewrCount = groupVoice.config().rewrite().size(); + std::string groupName = groupVoice.name(); + uint32_t tgId = groupVoice.source().tgId(); + uint8_t tgSlot = groupVoice.source().tgSlot(); + bool active = groupVoice.config().active(); + bool parrot = groupVoice.config().parrot(); - if (incCount > 0 && excCount > 0) { - ::LogWarning(LOG_HOST, "Talkgroup (%s) defines both inclusions and exclusions! Inclusions take precedence and exclusions will be ignored.", groupName.c_str()); - } + uint32_t incCount = groupVoice.config().inclusion().size(); + uint32_t excCount = groupVoice.config().exclusion().size(); + uint32_t rewrCount = groupVoice.config().rewrite().size(); - ::LogInfoEx(LOG_HOST, "Talkgroup NAME: %s SRC_TGID: %u SRC_TS: %u ACTIVE: %u PARROT: %u INCLUSIONS: %u EXCLUSIONS: %u REWRITES: %u", groupName.c_str(), tgId, tgSlot, active, parrot, incCount, excCount, rewrCount); + if (incCount > 0 && excCount > 0) { + ::LogWarning(LOG_HOST, "Talkgroup (%s) defines both inclusions and exclusions! Inclusions take precedence and exclusions will be ignored.", groupName.c_str()); } + + ::LogInfoEx(LOG_HOST, "Talkgroup NAME: %s SRC_TGID: %u SRC_TS: %u ACTIVE: %u PARROT: %u INCLUSIONS: %u EXCLUSIONS: %u REWRITES: %u", groupName.c_str(), tgId, tgSlot, active, parrot, incCount, excCount, rewrCount); } - m_mutex.unlock(); size_t size = m_groupVoice.size(); if (size == 0U) @@ -388,7 +373,7 @@ bool TalkgroupRulesLookup::save() return false; } - m_mutex.lock(); + std::lock_guard lock(m_mutex); // New list for our new group voice rules yaml::Node groupVoiceList; @@ -405,8 +390,6 @@ bool TalkgroupRulesLookup::save() // Set the new rules newRules["groupVoice"] = groupVoiceList; - m_mutex.unlock(); - // Make sure we actually did stuff right if (newRules["groupVoice"].size() != m_groupVoice.size()) { LogError(LOG_HOST, "Generated YAML node for group lists did not match loaded group size! (%u != %u)", newRules["groupVoice"].size(), m_groupVoice.size()); diff --git a/src/common/lookups/TalkgroupRulesLookup.h b/src/common/lookups/TalkgroupRulesLookup.h index 6687ee9c..e1ade8bc 100644 --- a/src/common/lookups/TalkgroupRulesLookup.h +++ b/src/common/lookups/TalkgroupRulesLookup.h @@ -371,7 +371,7 @@ namespace lookups bool m_acl; - std::mutex m_mutex; + static std::mutex m_mutex; bool m_stop; /// Loads the table from the passed lookup table file. diff --git a/src/common/network/RawFrameQueue.cpp b/src/common/network/RawFrameQueue.cpp index 2d28665a..be29db95 100644 --- a/src/common/network/RawFrameQueue.cpp +++ b/src/common/network/RawFrameQueue.cpp @@ -21,6 +21,12 @@ using namespace network; #include #include +// --------------------------------------------------------------------------- +// Static Class Members +// --------------------------------------------------------------------------- + +std::mutex RawFrameQueue::m_flushMutex; + // --------------------------------------------------------------------------- // Public Class Members // --------------------------------------------------------------------------- @@ -32,7 +38,6 @@ using namespace network; /// RawFrameQueue::RawFrameQueue(udp::Socket* socket, bool debug) : m_socket(socket), - m_flushMutex(), m_buffers(), m_debug(debug) { @@ -147,32 +152,27 @@ void RawFrameQueue::enqueueMessage(const uint8_t* message, uint32_t length, sock bool RawFrameQueue::flushQueue() { bool ret = true; - m_flushMutex.lock(); - { - if (m_buffers.empty()) { - m_flushMutex.unlock(); - return false; - } + std::lock_guard lock(m_flushMutex); - // bryanb: this is the same as above -- but for some assinine reason prevents - // weirdness - if (m_buffers.size() == 0U) { - m_flushMutex.unlock(); - return false; - } + if (m_buffers.empty()) { + return false; + } - // LogDebug(LOG_NET, "m_buffers len = %u", m_buffers.size()); + // bryanb: this is the same as above -- but for some assinine reason prevents + // weirdness + if (m_buffers.size() == 0U) { + return false; + } - ret = true; - if (!m_socket->write(m_buffers)) { - LogError(LOG_NET, "Failed writing data to the network"); - ret = false; - } + // LogDebug(LOG_NET, "m_buffers len = %u", m_buffers.size()); - deleteBuffers(); + ret = true; + if (!m_socket->write(m_buffers)) { + LogError(LOG_NET, "Failed writing data to the network"); + ret = false; } - m_flushMutex.unlock(); + deleteBuffers(); return ret; } diff --git a/src/common/network/RawFrameQueue.h b/src/common/network/RawFrameQueue.h index 58bdc524..3ee6502f 100644 --- a/src/common/network/RawFrameQueue.h +++ b/src/common/network/RawFrameQueue.h @@ -59,7 +59,7 @@ namespace network uint32_t m_addrLen; udp::Socket* m_socket; - std::mutex m_flushMutex; + static std::mutex m_flushMutex; udp::BufferVector m_buffers; bool m_debug; diff --git a/src/fne/network/FNENetwork.cpp b/src/fne/network/FNENetwork.cpp index 0a0bccd5..ad46ae4a 100644 --- a/src/fne/network/FNENetwork.cpp +++ b/src/fne/network/FNENetwork.cpp @@ -37,6 +37,12 @@ const uint32_t MAX_HARD_CONN_CAP = 250U; const uint8_t MAX_PEER_LIST_BEFORE_FLUSH = 10U; const uint32_t MAX_RID_LIST_CHUNK = 50U; +// --------------------------------------------------------------------------- +// Static Class Members +// --------------------------------------------------------------------------- + +std::mutex FNENetwork::m_peerMutex; + // --------------------------------------------------------------------------- // Public Class Members // --------------------------------------------------------------------------- @@ -80,7 +86,6 @@ FNENetwork::FNENetwork(HostFNE* host, const std::string& address, uint16_t port, m_ridLookup(nullptr), m_tidLookup(nullptr), m_status(NET_STAT_INVALID), - m_peerMutex(), m_peers(), m_peerAffiliations(), m_maintainenceTimer(1000U, pingTime), @@ -975,19 +980,15 @@ void* FNENetwork::threadedNetworkRx(void* arg) /// bool FNENetwork::erasePeerAffiliations(uint32_t peerId) { - m_peerMutex.lock(); - { - auto it = std::find_if(m_peerAffiliations.begin(), m_peerAffiliations.end(), [&](PeerAffiliationMapPair x) { return x.first == peerId; }); - if (it != m_peerAffiliations.end()) { - lookups::AffiliationLookup* aff = m_peerAffiliations[peerId]; - m_peerAffiliations.erase(peerId); - delete aff; - - m_peerMutex.unlock(); - return true; - } + std::lock_guard lock(m_peerMutex); + auto it = std::find_if(m_peerAffiliations.begin(), m_peerAffiliations.end(), [&](PeerAffiliationMapPair x) { return x.first == peerId; }); + if (it != m_peerAffiliations.end()) { + lookups::AffiliationLookup* aff = m_peerAffiliations[peerId]; + m_peerAffiliations.erase(peerId); + delete aff; + + return true; } - m_peerMutex.unlock(); return false; } @@ -999,16 +1000,12 @@ bool FNENetwork::erasePeerAffiliations(uint32_t peerId) /// bool FNENetwork::erasePeer(uint32_t peerId) { - m_peerMutex.lock(); - { - auto it = std::find_if(m_peers.begin(), m_peers.end(), [&](PeerMapPair x) { return x.first == peerId; }); - if (it != m_peers.end()) { - m_peers.erase(peerId); - m_peerMutex.unlock(); - return true; - } + std::lock_guard lock(m_peerMutex); + auto it = std::find_if(m_peers.begin(), m_peers.end(), [&](PeerMapPair x) { return x.first == peerId; }); + if (it != m_peers.end()) { + m_peers.erase(peerId); + return true; } - m_peerMutex.unlock(); return false; } diff --git a/src/fne/network/FNENetwork.h b/src/fne/network/FNENetwork.h index e652dee0..7392010a 100644 --- a/src/fne/network/FNENetwork.h +++ b/src/fne/network/FNENetwork.h @@ -270,7 +270,7 @@ namespace network NET_CONN_STATUS m_status; - std::mutex m_peerMutex; + static std::mutex m_peerMutex; typedef std::pair PeerMapPair; std::unordered_map m_peers; typedef std::pair PeerAffiliationMapPair; diff --git a/src/host/Host.cpp b/src/host/Host.cpp index 3711d633..28395037 100644 --- a/src/host/Host.cpp +++ b/src/host/Host.cpp @@ -714,7 +714,7 @@ int Host::run() } bool hasTxShutdown = false; - std::mutex clockingMutex; + static std::mutex clockingMutex; // Macro to start DMR duplex idle transmission (or beacon) #define START_DMR_DUPLEX_IDLE(x) \ @@ -734,8 +734,10 @@ int Host::run() if (dmr != nullptr) { LogDebug(LOG_HOST, "DMR, started slot 1 frame processor (modem write)"); while (!g_killed) { - clockingMutex.lock(); + // scope is intentional { + std::lock_guard lock(clockingMutex); + // ------------------------------------------------------ // -- Write to Modem Processing -- // ------------------------------------------------------ @@ -757,7 +759,6 @@ int Host::run() } }); } - clockingMutex.unlock(); if (m_state != STATE_IDLE) Thread::sleep(m_activeTickDelay); @@ -776,8 +777,10 @@ int Host::run() if (dmr != nullptr) { LogDebug(LOG_HOST, "DMR, started slot 2 frame processor (modem write)"); while (!g_killed) { - clockingMutex.lock(); + // scope is intentional { + std::lock_guard lock(clockingMutex); + // ------------------------------------------------------ // -- Write to Modem Processing -- // ------------------------------------------------------ @@ -799,7 +802,6 @@ int Host::run() } }); } - clockingMutex.unlock(); if (m_state != STATE_IDLE) Thread::sleep(m_activeTickDelay); @@ -819,8 +821,10 @@ int Host::run() if (p25 != nullptr) { LogDebug(LOG_HOST, "P25, started frame processor (modem write)"); while (!g_killed) { - clockingMutex.lock(); + // scope is intentional { + std::lock_guard lock(clockingMutex); + // ------------------------------------------------------ // -- Write to Modem Processing -- // ------------------------------------------------------ @@ -839,7 +843,6 @@ int Host::run() } }); } - clockingMutex.unlock(); if (m_state != STATE_IDLE) Thread::sleep(m_activeTickDelay); @@ -859,8 +862,10 @@ int Host::run() if (nxdn != nullptr) { LogDebug(LOG_HOST, "NXDN, started frame processor (modem write)"); while (!g_killed) { - clockingMutex.lock(); + // scope is intentional { + std::lock_guard lock(clockingMutex); + // ------------------------------------------------------ // -- Write to Modem Processing -- // ------------------------------------------------------ @@ -879,7 +884,6 @@ int Host::run() } }); } - clockingMutex.unlock(); if (m_state != STATE_IDLE) Thread::sleep(m_activeTickDelay); @@ -928,8 +932,10 @@ int Host::run() } } - clockingMutex.lock(); + // scope is intentional { + std::lock_guard lock(clockingMutex); + // ------------------------------------------------------ // -- Modem Clocking -- // ------------------------------------------------------ @@ -939,7 +945,6 @@ int Host::run() m_modem->clock(ms); } - clockingMutex.unlock(); // ------------------------------------------------------ // -- Read from Modem Processing -- @@ -1057,7 +1062,7 @@ int Host::run() LogDebug(LOG_HOST, "CW, start transmitting"); m_isTxCW = true; - clockingMutex.lock(); + std::lock_guard lock(clockingMutex); setState(STATE_IDLE); m_modem->sendCWId(m_cwCallsign); @@ -1088,7 +1093,6 @@ int Host::run() Thread::sleep(CW_IDLE_SLEEP_MS); } while (true); - clockingMutex.unlock(); m_isTxCW = false; m_cwIdTimer.setTimeout(m_cwIdTime); m_cwIdTimer.start();