From 5bb43f5aaab7fd393011d80ce303fa8351c42772 Mon Sep 17 00:00:00 2001 From: vsonnier Date: Thu, 2 Jun 2016 23:56:31 +0200 Subject: [PATCH] Replace mutex lock/unlock pairs with guards, cleanups --- src/AppConfig.cpp | 24 +-- src/AppFrame.cpp | 214 ++++++++++++------------ src/CubicSDR.cpp | 16 +- src/CubicSDR.h | 2 + src/IOThread.h | 6 + src/audio/AudioThread.h | 1 - src/demod/DemodDefs.h | 2 +- src/demod/DemodulatorInstance.cpp | 4 +- src/demod/DemodulatorMgr.cpp | 32 +++- src/demod/DemodulatorMgr.h | 4 +- src/process/SpectrumVisualProcessor.cpp | 49 +++--- src/process/SpectrumVisualProcessor.h | 1 + src/process/VisualProcessor.h | 31 +++- src/sdr/SDRPostThread.cpp | 9 +- src/sdr/SDRPostThread.h | 4 + src/sdr/SoapySDRThread.cpp | 53 +++--- src/visual/WaterfallCanvas.cpp | 12 +- 17 files changed, 265 insertions(+), 199 deletions(-) diff --git a/src/AppConfig.cpp b/src/AppConfig.cpp index 0c20f83..85b3016 100644 --- a/src/AppConfig.cpp +++ b/src/AppConfig.cpp @@ -47,39 +47,39 @@ bool DeviceConfig::getAGCMode() { void DeviceConfig::setDeviceId(std::string deviceId) { - busy_lock.lock(); + std::lock_guard < std::mutex > lock(busy_lock); this->deviceId = deviceId; - busy_lock.unlock(); + } std::string DeviceConfig::getDeviceId() { std::string tmp; - busy_lock.lock(); + std::lock_guard < std::mutex > lock(busy_lock); tmp = deviceId; - busy_lock.unlock(); + return tmp; } void DeviceConfig::setDeviceName(std::string deviceName) { - busy_lock.lock(); + std::lock_guard < std::mutex > lock(busy_lock); this->deviceName = deviceName; - busy_lock.unlock(); + } std::string DeviceConfig::getDeviceName() { std::string tmp; - busy_lock.lock(); + std::lock_guard < std::mutex > lock(busy_lock); tmp = (deviceName=="")?deviceId:deviceName; - busy_lock.unlock(); + return tmp; } void DeviceConfig::save(DataNode *node) { - busy_lock.lock(); + std::lock_guard < std::mutex > lock(busy_lock); *node->newChild("id") = deviceId; *node->newChild("name") = deviceName; *node->newChild("ppm") = (int)ppm.load(); @@ -115,11 +115,11 @@ void DeviceConfig::save(DataNode *node) { *gainNode->newChild("value") = gain_i->second; } } - busy_lock.unlock(); + } void DeviceConfig::load(DataNode *node) { - busy_lock.lock(); + std::lock_guard < std::mutex > lock(busy_lock); if (node->hasAnother("name")) { deviceName = node->getNext("name")->element()->toString(); } @@ -201,7 +201,7 @@ void DeviceConfig::load(DataNode *node) { } } } - busy_lock.unlock(); + } void DeviceConfig::setStreamOpts(ConfigSettings opts) { diff --git a/src/AppFrame.cpp b/src/AppFrame.cpp index 6e88450..2062eea 100644 --- a/src/AppFrame.cpp +++ b/src/AppFrame.cpp @@ -1604,16 +1604,16 @@ bool AppFrame::loadSession(std::string fileName) { DataNode *header = l.rootNode()->getNext("header"); if (header->hasAnother("version")) { - std::string version(*header->getNext("version")); + std::string version(*header->getNext("version")); // std::cout << "Loading " << version << " session file" << std::endl; } if (header->hasAnother("center_freq")) { - long long center_freq = *header->getNext("center_freq"); + long long center_freq = *header->getNext("center_freq"); // std::cout << "\tCenter Frequency: " << center_freq << std::endl; - wxGetApp().setFrequency(center_freq); - } + wxGetApp().setFrequency(center_freq); + } if (header->hasAnother("sample_rate")) { int sample_rate = *header->getNext("sample_rate"); @@ -1631,135 +1631,135 @@ bool AppFrame::loadSession(std::string fileName) { } if (l.rootNode()->hasAnother("demodulators")) { - DataNode *demodulators = l.rootNode()->getNext("demodulators"); + DataNode *demodulators = l.rootNode()->getNext("demodulators"); - int numDemodulators = 0; - DemodulatorInstance *loadedDemod = NULL; - DemodulatorInstance *newDemod = NULL; - std::vector demodsLoaded; + int numDemodulators = 0; + DemodulatorInstance *loadedDemod = NULL; + DemodulatorInstance *newDemod = NULL; + std::vector demodsLoaded; + + while (demodulators->hasAnother("demodulator")) { + DataNode *demod = demodulators->getNext("demodulator"); + + if (!demod->hasAnother("bandwidth") || !demod->hasAnother("frequency")) { + continue; + } + + long bandwidth = *demod->getNext("bandwidth"); + long long freq = *demod->getNext("frequency"); + float squelch_level = demod->hasAnother("squelch_level") ? (float) *demod->getNext("squelch_level") : 0; + int squelch_enabled = demod->hasAnother("squelch_enabled") ? (int) *demod->getNext("squelch_enabled") : 0; + int muted = demod->hasAnother("muted") ? (int) *demod->getNext("muted") : 0; + int delta_locked = demod->hasAnother("delta_lock") ? (int) *demod->getNext("delta_lock") : 0; + int delta_ofs = demod->hasAnother("delta_ofs") ? (int) *demod->getNext("delta_ofs") : 0; + std::string output_device = demod->hasAnother("output_device") ? string(*(demod->getNext("output_device"))) : ""; + float gain = demod->hasAnother("gain") ? (float) *demod->getNext("gain") : 1.0; - while (demodulators->hasAnother("demodulator")) { - DataNode *demod = demodulators->getNext("demodulator"); + std::string type = "FM"; - if (!demod->hasAnother("bandwidth") || !demod->hasAnother("frequency")) { - continue; + DataNode *demodTypeNode = demod->hasAnother("type")?demod->getNext("type"):nullptr; + + if (demodTypeNode && demodTypeNode->element()->getDataType() == DATA_INT) { + int legacyType = *demodTypeNode; + int legacyStereo = demod->hasAnother("stereo") ? (int) *demod->getNext("stereo") : 0; + switch (legacyType) { // legacy demod ID + case 1: type = legacyStereo?"FMS":"FM"; break; + case 2: type = "AM"; break; + case 3: type = "LSB"; break; + case 4: type = "USB"; break; + case 5: type = "DSB"; break; + case 6: type = "ASK"; break; + case 7: type = "APSK"; break; + case 8: type = "BPSK"; break; + case 9: type = "DPSK"; break; + case 10: type = "PSK"; break; + case 11: type = "OOK"; break; + case 12: type = "ST"; break; + case 13: type = "SQAM"; break; + case 14: type = "QAM"; break; + case 15: type = "QPSK"; break; + case 16: type = "I/Q"; break; + default: type = "FM"; break; } + } else if (demodTypeNode && demodTypeNode->element()->getDataType() == DATA_STRING) { + demodTypeNode->element()->get(type); + } - long bandwidth = *demod->getNext("bandwidth"); - long long freq = *demod->getNext("frequency"); - float squelch_level = demod->hasAnother("squelch_level") ? (float) *demod->getNext("squelch_level") : 0; - int squelch_enabled = demod->hasAnother("squelch_enabled") ? (int) *demod->getNext("squelch_enabled") : 0; - int muted = demod->hasAnother("muted") ? (int) *demod->getNext("muted") : 0; - int delta_locked = demod->hasAnother("delta_lock") ? (int) *demod->getNext("delta_lock") : 0; - int delta_ofs = demod->hasAnother("delta_ofs") ? (int) *demod->getNext("delta_ofs") : 0; - std::string output_device = demod->hasAnother("output_device") ? string(*(demod->getNext("output_device"))) : ""; - float gain = demod->hasAnother("gain") ? (float) *demod->getNext("gain") : 1.0; - - std::string type = "FM"; - - DataNode *demodTypeNode = demod->hasAnother("type")?demod->getNext("type"):nullptr; - - if (demodTypeNode && demodTypeNode->element()->getDataType() == DATA_INT) { - int legacyType = *demodTypeNode; - int legacyStereo = demod->hasAnother("stereo") ? (int) *demod->getNext("stereo") : 0; - switch (legacyType) { // legacy demod ID - case 1: type = legacyStereo?"FMS":"FM"; break; - case 2: type = "AM"; break; - case 3: type = "LSB"; break; - case 4: type = "USB"; break; - case 5: type = "DSB"; break; - case 6: type = "ASK"; break; - case 7: type = "APSK"; break; - case 8: type = "BPSK"; break; - case 9: type = "DPSK"; break; - case 10: type = "PSK"; break; - case 11: type = "OOK"; break; - case 12: type = "ST"; break; - case 13: type = "SQAM"; break; - case 14: type = "QAM"; break; - case 15: type = "QPSK"; break; - case 16: type = "I/Q"; break; - default: type = "FM"; break; - } - } else if (demodTypeNode && demodTypeNode->element()->getDataType() == DATA_STRING) { - demodTypeNode->element()->get(type); - } - - ModemSettings mSettings; - - if (demod->hasAnother("settings")) { - DataNode *modemSettings = demod->getNext("settings"); - for (int msi = 0, numSettings = modemSettings->numChildren(); msi < numSettings; msi++) { - DataNode *settingNode = modemSettings->child(msi); - std::string keyName = settingNode->getName(); - std::string strSettingValue = settingNode->element()->toString(); - - if (keyName != "" && strSettingValue != "") { - mSettings[keyName] = strSettingValue; - } + ModemSettings mSettings; + + if (demod->hasAnother("settings")) { + DataNode *modemSettings = demod->getNext("settings"); + for (int msi = 0, numSettings = modemSettings->numChildren(); msi < numSettings; msi++) { + DataNode *settingNode = modemSettings->child(msi); + std::string keyName = settingNode->getName(); + std::string strSettingValue = settingNode->element()->toString(); + + if (keyName != "" && strSettingValue != "") { + mSettings[keyName] = strSettingValue; } } - - newDemod = wxGetApp().getDemodMgr().newThread(); + } + + newDemod = wxGetApp().getDemodMgr().newThread(); - if (demod->hasAnother("active")) { - loadedDemod = newDemod; - } + if (demod->hasAnother("active")) { + loadedDemod = newDemod; + } - numDemodulators++; - newDemod->setDemodulatorType(type); - newDemod->writeModemSettings(mSettings); - newDemod->setBandwidth(bandwidth); - newDemod->setFrequency(freq); - newDemod->setGain(gain); - newDemod->updateLabel(freq); - newDemod->setMuted(muted?true:false); - if (delta_locked) { - newDemod->setDeltaLock(true); - newDemod->setDeltaLockOfs(delta_ofs); - } - if (squelch_enabled) { - newDemod->setSquelchEnabled(true); - newDemod->setSquelchLevel(squelch_level); - } - - bool found_device = false; - std::map::iterator i; - for (i = outputDevices.begin(); i != outputDevices.end(); i++) { - if (i->second.name == output_device) { - newDemod->setOutputDevice(i->first); - found_device = true; - } + numDemodulators++; + newDemod->setDemodulatorType(type); + newDemod->writeModemSettings(mSettings); + newDemod->setBandwidth(bandwidth); + newDemod->setFrequency(freq); + newDemod->setGain(gain); + newDemod->updateLabel(freq); + newDemod->setMuted(muted?true:false); + if (delta_locked) { + newDemod->setDeltaLock(true); + newDemod->setDeltaLockOfs(delta_ofs); + } + if (squelch_enabled) { + newDemod->setSquelchEnabled(true); + newDemod->setSquelchLevel(squelch_level); + } + + bool found_device = false; + std::map::iterator i; + for (i = outputDevices.begin(); i != outputDevices.end(); i++) { + if (i->second.name == output_device) { + newDemod->setOutputDevice(i->first); + found_device = true; } + } // if (!found_device) { // std::cout << "\tWarning: named output device '" << output_device << "' was not found. Using default output."; // } - newDemod->run(); - newDemod->setActive(true); - demodsLoaded.push_back(newDemod); - // wxGetApp().bindDemodulator(newDemod); + newDemod->run(); + newDemod->setActive(true); + demodsLoaded.push_back(newDemod); +// wxGetApp().bindDemodulator(newDemod); std::cout << "\tAdded demodulator at frequency " << newDemod->getFrequency() << " type " << type << std::endl; // std::cout << "\t\tBandwidth: " << bandwidth << std::endl; // std::cout << "\t\tSquelch Level: " << squelch_level << std::endl; // std::cout << "\t\tSquelch Enabled: " << (squelch_enabled ? "true" : "false") << std::endl; // std::cout << "\t\tOutput Device: " << output_device << std::endl; - } - - DemodulatorInstance *focusDemod = loadedDemod?loadedDemod:newDemod; - - if (focusDemod) { - wxGetApp().bindDemodulators(&demodsLoaded); - wxGetApp().getDemodMgr().setActiveDemodulator(focusDemod, false); - } + } + + DemodulatorInstance *focusDemod = loadedDemod?loadedDemod:newDemod; + + if (focusDemod) { + wxGetApp().bindDemodulators(&demodsLoaded); + wxGetApp().getDemodMgr().setActiveDemodulator(focusDemod, false); + } } } catch (DataTypeMismatchException &e) { std::cout << e.what() << std::endl; return false; } - + currentSessionFile = fileName; std::string filePart = fileName.substr(fileName.find_last_of(filePathSeparator) + 1); diff --git a/src/CubicSDR.cpp b/src/CubicSDR.cpp index 4e6ea4a..db3035b 100644 --- a/src/CubicSDR.cpp +++ b/src/CubicSDR.cpp @@ -397,7 +397,10 @@ void CubicSDR::removeRemote(std::string remoteAddr) { } void CubicSDR::sdrThreadNotify(SDRThread::SDRThreadState state, std::string message) { - notify_busy.lock(); + + std::lock_guard < std::mutex > lock(notify_busy); + + if (state == SDRThread::SDR_THREAD_INITIALIZED) { appframe->initDeviceParams(getDevice()); } @@ -415,12 +418,13 @@ void CubicSDR::sdrThreadNotify(SDRThread::SDRThreadState state, std::string mess // info->ShowModal(); } //if (appframe) { appframe->SetStatusText(message); } - notify_busy.unlock(); + } void CubicSDR::sdrEnumThreadNotify(SDREnumerator::SDREnumState state, std::string message) { - notify_busy.lock(); + std::lock_guard < std::mutex > lock(notify_busy); + if (state == SDREnumerator::SDR_ENUM_MESSAGE) { notifyMessage = message; } @@ -432,7 +436,7 @@ void CubicSDR::sdrEnumThreadNotify(SDREnumerator::SDREnumState state, std::strin devicesFailed.store(true); } //if (appframe) { appframe->SetStatusText(message); } - notify_busy.unlock(); + } @@ -748,9 +752,9 @@ bool CubicSDR::areModulesMissing() { std::string CubicSDR::getNotification() { std::string msg; - notify_busy.lock(); + std::lock_guard < std::mutex > lock(notify_busy); msg = notifyMessage; - notify_busy.unlock(); + return msg; } diff --git a/src/CubicSDR.h b/src/CubicSDR.h index 1641669..a6e261f 100644 --- a/src/CubicSDR.h +++ b/src/CubicSDR.h @@ -212,7 +212,9 @@ private: std::atomic_bool useLocalMod; std::string notifyMessage; std::string modulePath; + std::mutex notify_busy; + std::atomic_bool frequency_locked; std::atomic_llong lock_freq; FrequencyDialog::FrequencyDialogTarget fdlgTarget; diff --git a/src/IOThread.h b/src/IOThread.h index 9d9232b..ee93f6f 100644 --- a/src/IOThread.h +++ b/src/IOThread.h @@ -37,6 +37,12 @@ public: std::lock_guard < std::recursive_mutex > lock(m_mutex); return refCount; } + + // Access to the own mutex protecting the ReferenceCounter, i.e the monitor of the class + std::recursive_mutex& getMonitor() const { + return m_mutex; + } + protected: //this is a basic mutex for all ReferenceCounter derivatives operations INCLUDING the counter itself for consistency ! mutable std::recursive_mutex m_mutex; diff --git a/src/audio/AudioThread.h b/src/audio/AudioThread.h index 57193f5..97faaa1 100644 --- a/src/audio/AudioThread.h +++ b/src/audio/AudioThread.h @@ -20,7 +20,6 @@ public: float peak; int type; std::vector data; - std::mutex busy_update; AudioThreadInput() : frequency(0), sampleRate(0), channels(0), peak(0) { diff --git a/src/demod/DemodDefs.h b/src/demod/DemodDefs.h index 8dc5163..5a59e03 100644 --- a/src/demod/DemodDefs.h +++ b/src/demod/DemodDefs.h @@ -53,7 +53,7 @@ public: long long frequency; long long sampleRate; std::vector data; - std::mutex busy_rw; + DemodulatorThreadIQData() : frequency(0), sampleRate(0) { diff --git a/src/demod/DemodulatorInstance.cpp b/src/demod/DemodulatorInstance.cpp index 3519660..867f921 100644 --- a/src/demod/DemodulatorInstance.cpp +++ b/src/demod/DemodulatorInstance.cpp @@ -11,9 +11,9 @@ DemodulatorInstance::DemodulatorInstance() : #if ENABLE_DIGITAL_LAB activeOutput = nullptr; #endif - terminated.store(true); + terminated.store(true); demodTerminated.store(true); - audioTerminated.store(true); + audioTerminated.store(true); preDemodTerminated.store(true); active.store(false); squelch.store(false); diff --git a/src/demod/DemodulatorMgr.cpp b/src/demod/DemodulatorMgr.cpp index f28f144..95970be 100644 --- a/src/demod/DemodulatorMgr.cpp +++ b/src/demod/DemodulatorMgr.cpp @@ -31,7 +31,7 @@ DemodulatorMgr::~DemodulatorMgr() { } DemodulatorInstance *DemodulatorMgr::newThread() { - std::lock_guard < std::mutex > lock(demods_busy); + std::lock_guard < std::recursive_mutex > lock(demods_busy); DemodulatorInstance *newDemod = new DemodulatorInstance; std::stringstream label; @@ -44,8 +44,9 @@ DemodulatorInstance *DemodulatorMgr::newThread() { } void DemodulatorMgr::terminateAll() { - std::lock_guard < std::mutex > lock(demods_busy); + std::lock_guard < std::recursive_mutex > lock(demods_busy); while (demods.size()) { + DemodulatorInstance *d = demods.back(); demods.pop_back(); wxGetApp().removeDemodulator(d); @@ -54,11 +55,12 @@ void DemodulatorMgr::terminateAll() { } std::vector &DemodulatorMgr::getDemodulators() { - std::lock_guard < std::mutex > lock(demods_busy); + std::lock_guard < std::recursive_mutex > lock(demods_busy); return demods; } std::vector DemodulatorMgr::getOrderedDemodulators(bool actives) { + std::lock_guard < std::recursive_mutex > lock(demods_busy); std::vector demods_ordered = demods; if (actives) { std::sort(demods_ordered.begin(), demods_ordered.end(), inactiveCompare); @@ -74,11 +76,13 @@ std::vector DemodulatorMgr::getOrderedDemodulators(bool a demods_ordered.erase(demods_ordered.begin(), i); } } - std::sort(demods_ordered.begin(), demods_ordered.end(), demodFreqCompare); + //if by chance they have the same frequency, keep their relative order + std::stable_sort(demods_ordered.begin(), demods_ordered.end(), demodFreqCompare); return demods_ordered; } DemodulatorInstance *DemodulatorMgr::getPreviousDemodulator(DemodulatorInstance *demod, bool actives) { + std::lock_guard < std::recursive_mutex > lock(demods_busy); if (!getLastActiveDemodulator()) { return nullptr; } @@ -94,6 +98,7 @@ DemodulatorInstance *DemodulatorMgr::getPreviousDemodulator(DemodulatorInstance } DemodulatorInstance *DemodulatorMgr::getNextDemodulator(DemodulatorInstance *demod, bool actives) { + std::lock_guard < std::recursive_mutex > lock(demods_busy); if (!getLastActiveDemodulator()) { return nullptr; } @@ -112,16 +117,20 @@ DemodulatorInstance *DemodulatorMgr::getNextDemodulator(DemodulatorInstance *dem } DemodulatorInstance *DemodulatorMgr::getLastDemodulator() { + std::lock_guard < std::recursive_mutex > lock(demods_busy); std::vector demods_ordered = getOrderedDemodulators(); return *(demods_ordered.end()); } DemodulatorInstance *DemodulatorMgr::getFirstDemodulator() { + std::lock_guard < std::recursive_mutex > lock(demods_busy); std::vector demods_ordered = getOrderedDemodulators(); return *(demods_ordered.begin()); } void DemodulatorMgr::deleteThread(DemodulatorInstance *demod) { + std::lock_guard < std::recursive_mutex > lock(demods_busy); + std::vector::iterator i; i = std::find(demods.begin(), demods.end(), demod); @@ -145,7 +154,7 @@ void DemodulatorMgr::deleteThread(DemodulatorInstance *demod) { } std::vector *DemodulatorMgr::getDemodulatorsAt(long long freq, int bandwidth) { - std::lock_guard < std::mutex > lock(demods_busy); + std::lock_guard < std::recursive_mutex > lock(demods_busy); std::vector *foundDemods = new std::vector(); for (int i = 0, iMax = demods.size(); i < iMax; i++) { @@ -166,7 +175,7 @@ std::vector *DemodulatorMgr::getDemodulatorsAt(long long } bool DemodulatorMgr::anyDemodulatorsAt(long long freq, int bandwidth) { - std::lock_guard < std::mutex > lock(demods_busy); + std::lock_guard < std::recursive_mutex > lock(demods_busy); for (int i = 0, iMax = demods.size(); i < iMax; i++) { DemodulatorInstance *testDemod = demods[i]; @@ -177,15 +186,17 @@ bool DemodulatorMgr::anyDemodulatorsAt(long long freq, int bandwidth) { long long halfBuffer = bandwidth / 2; if ((freq <= (freqTest + ((testDemod->getDemodulatorType() != "LSB")?halfBandwidthTest:0) + halfBuffer)) && (freq >= (freqTest - ((testDemod->getDemodulatorType() != "USB")?halfBandwidthTest:0) - halfBuffer))) { + return true; } } - + return false; } void DemodulatorMgr::setActiveDemodulator(DemodulatorInstance *demod, bool temporary) { + std::lock_guard < std::recursive_mutex > lock(demods_busy); if (!temporary) { if (activeDemodulator != NULL) { lastActiveDemodulator = activeDemodulator; @@ -218,6 +229,7 @@ void DemodulatorMgr::setActiveDemodulator(DemodulatorInstance *demod, bool tempo } activeDemodulator = demod; + } DemodulatorInstance *DemodulatorMgr::getActiveDemodulator() { @@ -231,9 +243,10 @@ DemodulatorInstance *DemodulatorMgr::getLastActiveDemodulator() { return lastActiveDemodulator; } +//Private internal method, no need to protect it with demods_busy void DemodulatorMgr::garbageCollect() { - std::lock_guard < std::mutex > lock(demods_busy); if (demods_deleted.size()) { + std::vector::iterator i; for (i = demods_deleted.begin(); i != demods_deleted.end(); i++) { @@ -245,13 +258,16 @@ void DemodulatorMgr::garbageCollect() { delete deleted; + return; } } + } } void DemodulatorMgr::updateLastState() { + std::lock_guard < std::recursive_mutex > lock(demods_busy); if (std::find(demods.begin(), demods.end(), lastActiveDemodulator) == demods.end()) { if (activeDemodulator && activeDemodulator->isActive()) { lastActiveDemodulator = activeDemodulator; diff --git a/src/demod/DemodulatorMgr.h b/src/demod/DemodulatorMgr.h index 8218045..40d8d76 100644 --- a/src/demod/DemodulatorMgr.h +++ b/src/demod/DemodulatorMgr.h @@ -72,7 +72,9 @@ private: bool lastMuted; bool lastDeltaLock; - std::mutex demods_busy; + //protects access to demods lists and such, need to be recursive + //because of the usage of public re-entrant methods + std::recursive_mutex demods_busy; std::map lastModemSettings; }; diff --git a/src/process/SpectrumVisualProcessor.cpp b/src/process/SpectrumVisualProcessor.cpp index 2a27630..16dfef7 100644 --- a/src/process/SpectrumVisualProcessor.cpp +++ b/src/process/SpectrumVisualProcessor.cpp @@ -54,24 +54,27 @@ bool SpectrumVisualProcessor::isView() { } void SpectrumVisualProcessor::setView(bool bView) { - busy_run.lock(); + + std::lock_guard < std::mutex > busy_lock(busy_run); is_view.store(bView); - busy_run.unlock(); + } void SpectrumVisualProcessor::setView(bool bView, long long centerFreq_in, long bandwidth_in) { - busy_run.lock(); + + std::lock_guard < std::mutex > busy_lock(busy_run); is_view.store(bView); bandwidth.store(bandwidth_in); centerFreq.store(centerFreq_in); - busy_run.unlock(); + } void SpectrumVisualProcessor::setFFTAverageRate(float fftAverageRate) { - busy_run.lock(); + + std::lock_guard < std::mutex > busy_lock(busy_run); this->fft_average_rate.store(fftAverageRate); - busy_run.unlock(); + } float SpectrumVisualProcessor::getFFTAverageRate() { @@ -79,9 +82,10 @@ float SpectrumVisualProcessor::getFFTAverageRate() { } void SpectrumVisualProcessor::setCenterFrequency(long long centerFreq_in) { - busy_run.lock(); + + std::lock_guard < std::mutex > busy_lock(busy_run); centerFreq.store(centerFreq_in); - busy_run.unlock(); + } long long SpectrumVisualProcessor::getCenterFrequency() { @@ -89,9 +93,10 @@ long long SpectrumVisualProcessor::getCenterFrequency() { } void SpectrumVisualProcessor::setBandwidth(long bandwidth_in) { - busy_run.lock(); + + std::lock_guard < std::mutex > busy_lock(busy_run); bandwidth.store(bandwidth_in); - busy_run.unlock(); + } long SpectrumVisualProcessor::getBandwidth() { @@ -99,6 +104,7 @@ long SpectrumVisualProcessor::getBandwidth() { } void SpectrumVisualProcessor::setPeakHold(bool peakHold_in) { + if (peakHold.load() && peakHold_in) { peakReset.store(PEAK_RESET_COUNT); } else { @@ -116,7 +122,8 @@ int SpectrumVisualProcessor::getDesiredInputSize() { } void SpectrumVisualProcessor::setup(unsigned int fftSize_in) { - busy_run.lock(); + + std::lock_guard < std::mutex > busy_lock(busy_run); fftSize = fftSize_in; fftSizeInternal = fftSize_in * SPECTRUM_VZM; @@ -190,7 +197,6 @@ void SpectrumVisualProcessor::setup(unsigned int fftSize_in) { fftPlan = fft_create_plan(fftSizeInternal, fftInput, fftOutput, LIQUID_FFT_FORWARD, 0); #endif - busy_run.unlock(); } void SpectrumVisualProcessor::setFFTSize(unsigned int fftSize_in) { @@ -234,9 +240,16 @@ void SpectrumVisualProcessor::process() { if (!iqData) { return; } - - iqData->busy_rw.lock(); - busy_run.lock(); + + + //Start by locking concurrent access to iqData + std::lock_guard < std::recursive_mutex > lock(iqData->getMonitor()); + + //then get the busy_lock + std::lock_guard < std::mutex > busy_lock(busy_run); + + + bool doPeak = peakHold.load() && (peakReset.load() == 0); if (fft_result.size() != fftSizeInternal) { @@ -275,8 +288,7 @@ void SpectrumVisualProcessor::process() { if (is_view.load()) { if (!iqData->frequency || !iqData->sampleRate) { iqData->decRefCount(); - iqData->busy_rw.unlock(); - busy_run.unlock(); + return; } @@ -706,8 +718,7 @@ void SpectrumVisualProcessor::process() { } iqData->decRefCount(); - iqData->busy_rw.unlock(); - busy_run.unlock(); + lastView = is_view.load(); } diff --git a/src/process/SpectrumVisualProcessor.h b/src/process/SpectrumVisualProcessor.h index 59e9236..35c99eb 100644 --- a/src/process/SpectrumVisualProcessor.h +++ b/src/process/SpectrumVisualProcessor.h @@ -97,6 +97,7 @@ private: std::vector shiftBuffer; std::vector resampleBuffer; std::atomic_int desiredInputSize; + std::mutex busy_run; std::atomic_bool hideDC, peakHold; std::atomic_int peakReset; diff --git a/src/process/VisualProcessor.h b/src/process/VisualProcessor.h index 345224b..a2a33a5 100644 --- a/src/process/VisualProcessor.h +++ b/src/process/VisualProcessor.h @@ -13,10 +13,14 @@ public: } bool isInputEmpty() { + std::lock_guard < std::recursive_mutex > busy_lock(busy_update); + return input->empty(); } bool isOutputEmpty() { + std::lock_guard < std::recursive_mutex > busy_lock(busy_update); + for (outputs_i = outputs.begin(); outputs_i != outputs.end(); outputs_i++) { if ((*outputs_i)->full()) { return false; @@ -26,6 +30,8 @@ public: } bool isAnyOutputEmpty() { + std::lock_guard < std::recursive_mutex > busy_lock(busy_update); + for (outputs_i = outputs.begin(); outputs_i != outputs.end(); outputs_i++) { if (!(*outputs_i)->full()) { return true; @@ -35,34 +41,37 @@ public: } void setInput(ThreadQueue *vis_in) { - busy_update.lock(); + std::lock_guard < std::recursive_mutex > busy_lock(busy_update); input = vis_in; - busy_update.unlock(); + } void attachOutput(ThreadQueue *vis_out) { // attach an output queue - busy_update.lock(); + std::lock_guard < std::recursive_mutex > busy_lock(busy_update); outputs.push_back(vis_out); - busy_update.unlock(); + } void removeOutput(ThreadQueue *vis_out) { // remove an output queue - busy_update.lock(); + std::lock_guard < std::recursive_mutex > busy_lock(busy_update); + typename std::vector *>::iterator i = std::find(outputs.begin(), outputs.end(), vis_out); if (i != outputs.end()) { outputs.erase(i); } - busy_update.unlock(); + } void run() { - busy_update.lock(); + + std::lock_guard < std::recursive_mutex > busy_lock(busy_update); + if (input && !input->empty()) { process(); } - busy_update.unlock(); + } protected: @@ -73,6 +82,8 @@ protected: void distribute(OutputDataType *output) { // distribute outputs + std::lock_guard < std::recursive_mutex > busy_lock(busy_update); + output->setRefCount(outputs.size()); for (outputs_i = outputs.begin(); outputs_i != outputs.end(); outputs_i++) { if ((*outputs_i)->full()) { @@ -86,7 +97,9 @@ protected: ThreadQueue *input; std::vector *> outputs; typename std::vector *>::iterator outputs_i; - std::mutex busy_update; + + //protects input and outputs, must be recursive because ao reentrance + std::recursive_mutex busy_update; }; diff --git a/src/sdr/SDRPostThread.cpp b/src/sdr/SDRPostThread.cpp index 055501e..59dd27f 100644 --- a/src/sdr/SDRPostThread.cpp +++ b/src/sdr/SDRPostThread.cpp @@ -27,9 +27,12 @@ SDRPostThread::~SDRPostThread() { } void SDRPostThread::bindDemodulator(DemodulatorInstance *demod) { + std::lock_guard < std::mutex > lock(busy_demod); + demodulators.push_back(demod); doRefresh.store(true); + } void SDRPostThread::bindDemodulators(std::vector *demods) { @@ -37,10 +40,12 @@ void SDRPostThread::bindDemodulators(std::vector *demods) return; } std::lock_guard < std::mutex > lock(busy_demod); + for (std::vector::iterator di = demods->begin(); di != demods->end(); di++) { demodulators.push_back(*di); doRefresh.store(true); } + } void SDRPostThread::removeDemodulator(DemodulatorInstance *demod) { @@ -49,12 +54,14 @@ void SDRPostThread::removeDemodulator(DemodulatorInstance *demod) { } std::lock_guard < std::mutex > lock(busy_demod); + std::vector::iterator i = std::find(demodulators.begin(), demodulators.end(), demod); if (i != demodulators.end()) { demodulators.erase(i); doRefresh.store(true); } + } void SDRPostThread::initPFBChannelizer() { @@ -79,7 +86,7 @@ void SDRPostThread::updateActiveDemodulators() { nRunDemods = 0; long long centerFreq = wxGetApp().getFrequency(); - + for (demod_i = demodulators.begin(); demod_i != demodulators.end(); demod_i++) { DemodulatorInstance *demod = *demod_i; DemodulatorThreadInputQueue *demodQueue = demod->getIQInputDataPipe(); diff --git a/src/sdr/SDRPostThread.h b/src/sdr/SDRPostThread.h index f6634e7..ad396b1 100644 --- a/src/sdr/SDRPostThread.h +++ b/src/sdr/SDRPostThread.h @@ -29,10 +29,14 @@ protected: DemodulatorThreadInputQueue *iqVisualQueue; DemodulatorThreadInputQueue *iqActiveDemodVisualQueue; + //protects access to demodulators lists and such std::mutex busy_demod; std::vector demodulators; + + private: + void initPFBChannelizer(); void updateActiveDemodulators(); void updateChannels(); diff --git a/src/sdr/SoapySDRThread.cpp b/src/sdr/SoapySDRThread.cpp index e3d121e..d3a1620 100644 --- a/src/sdr/SoapySDRThread.cpp +++ b/src/sdr/SoapySDRThread.cpp @@ -126,20 +126,22 @@ void SDRThread::init() { settingChanged.erase(settingChanged.begin(), settingChanged.end()); } - setting_busy.lock(); - for (settings_i = settingsInfo.begin(); settings_i != settingsInfo.end(); settings_i++) { - SoapySDR::ArgInfo setting = (*settings_i); - if ((settingChanged.find(setting.key) != settingChanged.end()) && (settings.find(setting.key) != settings.end())) { - device->writeSetting(setting.key, settings[setting.key]); - settingChanged[setting.key] = false; - } else { - settings[setting.key] = device->readSetting(setting.key); - settingChanged[setting.key] = false; - } - } - setting_value_changed.store(false); + { //enter scoped-lock + std::lock_guard < std::mutex > lock(setting_busy); - setting_busy.unlock(); + for (settings_i = settingsInfo.begin(); settings_i != settingsInfo.end(); settings_i++) { + SoapySDR::ArgInfo setting = (*settings_i); + if ((settingChanged.find(setting.key) != settingChanged.end()) && (settings.find(setting.key) != settings.end())) { + device->writeSetting(setting.key, settings[setting.key]); + settingChanged[setting.key] = false; + } else { + settings[setting.key] = device->readSetting(setting.key); + settingChanged[setting.key] = false; + } + } + setting_value_changed.store(false); + + } //leave lock guard scope updateSettings(); @@ -316,21 +318,22 @@ void SDRThread::updateSettings() { } if (gain_value_changed.load() && !agc_mode.load()) { - gain_busy.lock(); + std::lock_guard < std::mutex > lock(gain_busy); + for (std::map::iterator gci = gainChanged.begin(); gci != gainChanged.end(); gci++) { if (gci->second) { device->setGain(SOAPY_SDR_RX, 0, gci->first, gainValues[gci->first]); gainChanged[gci->first] = false; } } - gain_busy.unlock(); gain_value_changed.store(false); } if (setting_value_changed.load()) { - setting_busy.lock(); + + std::lock_guard < std::mutex > lock(setting_busy); for (std::map::iterator sci = settingChanged.begin(); sci != settingChanged.end(); sci++) { if (sci->second) { @@ -340,7 +343,6 @@ void SDRThread::updateSettings() { } setting_value_changed.store(false); - setting_busy.unlock(); doUpdate = true; } @@ -511,11 +513,10 @@ bool SDRThread::getIQSwap() { } void SDRThread::setGain(std::string name, float value) { - gain_busy.lock(); + std::lock_guard < std::mutex > lock(gain_busy); gainValues[name] = value; gainChanged[name] = true; gain_value_changed.store(true); - gain_busy.unlock(); DeviceConfig *devConfig = deviceConfig.load(); if (devConfig) { @@ -524,28 +525,30 @@ void SDRThread::setGain(std::string name, float value) { } float SDRThread::getGain(std::string name) { - gain_busy.lock(); + std::lock_guard < std::mutex > lock(gain_busy); float val = gainValues[name]; - gain_busy.unlock(); + return val; } void SDRThread::writeSetting(std::string name, std::string value) { - setting_busy.lock(); + + std::lock_guard < std::mutex > lock(setting_busy); + settings[name] = value; settingChanged[name] = true; setting_value_changed.store(true); if (deviceConfig.load() != nullptr) { deviceConfig.load()->setSetting(name, value); } - setting_busy.unlock(); } std::string SDRThread::readSetting(std::string name) { std::string val; - setting_busy.lock(); + std::lock_guard < std::mutex > lock(setting_busy); + val = device->readSetting(name); - setting_busy.unlock(); + return val; } diff --git a/src/visual/WaterfallCanvas.cpp b/src/visual/WaterfallCanvas.cpp index 51c53b2..8003345 100644 --- a/src/visual/WaterfallCanvas.cpp +++ b/src/visual/WaterfallCanvas.cpp @@ -83,7 +83,7 @@ void WaterfallCanvas::attachSpectrumCanvas(SpectrumCanvas *canvas_in) { } void WaterfallCanvas::processInputQueue() { - tex_update.lock(); + std::lock_guard < std::mutex > lock(tex_update); gTimer.update(); @@ -118,11 +118,11 @@ void WaterfallCanvas::processInputQueue() { glContext->SetCurrent(*this); waterfallPanel.update(); } - tex_update.unlock(); + } void WaterfallCanvas::OnPaint(wxPaintEvent& WXUNUSED(event)) { - tex_update.lock(); + std::lock_guard < std::mutex > lock(tex_update); wxPaintDC dc(this); const wxSize ClientSize = GetClientSize(); @@ -341,7 +341,6 @@ void WaterfallCanvas::OnPaint(wxPaintEvent& WXUNUSED(event)) { glContext->EndDraw(); SwapBuffers(); - tex_update.unlock(); } void WaterfallCanvas::OnKeyUp(wxKeyEvent& event) { @@ -905,17 +904,16 @@ void WaterfallCanvas::updateCenterFrequency(long long freq) { } void WaterfallCanvas::setLinesPerSecond(int lps) { - tex_update.lock(); + std::lock_guard < std::mutex > lock(tex_update); linesPerSecond = lps; while (!visualDataQueue.empty()) { SpectrumVisualData *vData; visualDataQueue.pop(vData); - + if (vData) { vData->decRefCount(); } } - tex_update.unlock(); } void WaterfallCanvas::setMinBandwidth(int min) {