From d8ac9559fe281c43b4207af8daae9378a2de6021 Mon Sep 17 00:00:00 2001 From: vsonnier Date: Tue, 16 Jan 2018 20:21:16 +0100 Subject: [PATCH] BoolmarkMgr: rationalize a bit the usage of recursive_mutexes --- src/BookmarkMgr.cpp | 80 ++++++++++++++++------------- src/BookmarkMgr.h | 30 ++++++----- src/forms/Bookmark/BookmarkView.cpp | 24 ++------- src/forms/Bookmark/BookmarkView.h | 1 - 4 files changed, 67 insertions(+), 68 deletions(-) diff --git a/src/BookmarkMgr.cpp b/src/BookmarkMgr.cpp index 85c4f86..edc13e6 100644 --- a/src/BookmarkMgr.cpp +++ b/src/BookmarkMgr.cpp @@ -15,12 +15,11 @@ BookmarkEntry::~BookmarkEntry() { BookmarkMgr::BookmarkMgr() { rangesSorted = false; } - -//represents an empty BookMarkList that is returned by reference by some functions. -const BookmarkList BookmarkMgr::emptyResults; - +// void BookmarkMgr::saveToFile(std::string bookmarkFn, bool backup, bool useFullpath) { + std::lock_guard < std::recursive_mutex > lock(busy_lock); + DataTree s("cubicsdr_bookmarks"); DataNode *header = s.rootNode()->newChild("header"); header->newChild("version")->element()->set(wxString(CUBICSDR_VERSION).ToStdWstring()); @@ -100,6 +99,8 @@ void BookmarkMgr::saveToFile(std::string bookmarkFn, bool backup, bool useFullpa } bool BookmarkMgr::loadFromFile(std::string bookmarkFn, bool backup, bool useFullpath) { + + std::lock_guard < std::recursive_mutex > lock(busy_lock); wxFileName loadFile; wxFileName failFile; @@ -129,7 +130,7 @@ bool BookmarkMgr::loadFromFile(std::string bookmarkFn, bool backup, bool useFull // New instance of bookmark savefiles if (backup && !loadFile.FileExists() && !lastLoaded.FileExists() && !backupFile.FileExists()) { - wxGetApp().getAppFrame()->getBookmarkView()->loadDefaultRanges(); + loadDefaultRanges(); return true; } @@ -145,8 +146,8 @@ bool BookmarkMgr::loadFromFile(std::string bookmarkFn, bool backup, bool useFull // Clear any active data bmData.clear(); - clearRecents(); - clearRanges(); + recents.clear(); + ranges.clear(); bmDataSorted.clear(); if (s.rootNode()->hasAnother("branches")) { @@ -238,15 +239,31 @@ bool BookmarkMgr::loadFromFile(std::string bookmarkFn, bool backup, bool useFull return loadStatusOk; } +void BookmarkMgr::loadDefaultRanges() { + + addRange(std::make_shared(L"160 Meters", 1900000, 1800000, 2000000)); + addRange(std::make_shared(L"80 Meters", 3750000, 3500000, 4000000)); + addRange(std::make_shared(L"60 Meters", 5368500, 5332000, 5405000)); + addRange(std::make_shared(L"40 Meters", 7150000, 7000000, 7300000)); + addRange(std::make_shared(L"30 Meters", 10125000, 10100000, 10150000)); + addRange(std::make_shared(L"20 Meters", 14175000, 14000000, 14350000)); + addRange(std::make_shared(L"17 Meters", 18068180, 17044180, 19092180)); + addRange(std::make_shared(L"15 Meters", 21225000, 21000000, 21450000)); + addRange(std::make_shared(L"12 Meters", 24940000, 24890000, 24990000)); + addRange(std::make_shared(L"10 Meters", 28850000, 28000000, 29700000)); +} + void BookmarkMgr::resetBookmarks() { + std::lock_guard < std::recursive_mutex > lock(busy_lock); + // Clear any active data bmData.clear(); - clearRecents(); - clearRanges(); + recents.clear(); + ranges.clear(); bmDataSorted.clear(); - wxGetApp().getAppFrame()->getBookmarkView()->loadDefaultRanges(); + loadDefaultRanges(); } @@ -280,9 +297,8 @@ void BookmarkMgr::addBookmark(std::string group, BookmarkEntryPtr be) { void BookmarkMgr::removeBookmark(std::string group, BookmarkEntryPtr be) { - std::lock_guard < std::recursive_mutex > lockData(busy_lock); - std::lock_guard < std::mutex > lockEnt(be->busy_lock); - + std::lock_guard < std::recursive_mutex > lock(busy_lock); + if (bmData.find(group) == bmData.end()) { return; } @@ -296,8 +312,7 @@ void BookmarkMgr::removeBookmark(std::string group, BookmarkEntryPtr be) { } void BookmarkMgr::removeBookmark(BookmarkEntryPtr be) { - std::lock_guard < std::recursive_mutex > lockData(busy_lock); - std::lock_guard < std::mutex > lockEnt(be->busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); for (auto &bmd_i : bmData) { BookmarkList::iterator i = std::find(bmd_i.second.begin(), bmd_i.second.end(), be); @@ -308,8 +323,7 @@ void BookmarkMgr::removeBookmark(BookmarkEntryPtr be) { } void BookmarkMgr::moveBookmark(BookmarkEntryPtr be, std::string group) { - std::lock_guard < std::recursive_mutex > lockData(busy_lock); - std::lock_guard < std::mutex > lockEnt(be->busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); for (auto &bmd_i : bmData) { BookmarkList::iterator i = std::find(bmd_i.second.begin(), bmd_i.second.end(), be); @@ -367,11 +381,11 @@ void BookmarkMgr::renameGroup(std::string group, std::string ngroup) { } } -const BookmarkList& BookmarkMgr::getBookmarks(std::string group) { +BookmarkList BookmarkMgr::getBookmarks(std::string group) { std::lock_guard < std::recursive_mutex > lock(busy_lock); if (bmData.find(group) == bmData.end()) { - return emptyResults; + return BookmarkList(); } if (!bmDataSorted[group]) { @@ -384,7 +398,7 @@ const BookmarkList& BookmarkMgr::getBookmarks(std::string group) { void BookmarkMgr::getGroups(BookmarkNames &arr) { - std::lock_guard < std::recursive_mutex > lockData(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); for (BookmarkMap::iterator i = bmData.begin(); i!= bmData.end(); ++i) { arr.push_back(i->first); @@ -392,7 +406,7 @@ void BookmarkMgr::getGroups(BookmarkNames &arr) { } void BookmarkMgr::getGroups(wxArrayString &arr) { - std::lock_guard < std::recursive_mutex > lockData(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); for (BookmarkMap::iterator i = bmData.begin(); i!= bmData.end(); ++i) { arr.push_back(i->first); @@ -401,11 +415,16 @@ void BookmarkMgr::getGroups(wxArrayString &arr) { void BookmarkMgr::setExpandState(std::string groupName, bool state) { + std::lock_guard < std::recursive_mutex > lock(busy_lock); + expandState[groupName] = state; } bool BookmarkMgr::getExpandState(std::string groupName) { + + std::lock_guard < std::recursive_mutex > lock(busy_lock); + if (expandState.find(groupName) == expandState.end()) { return true; } @@ -415,8 +434,6 @@ bool BookmarkMgr::getExpandState(std::string groupName) { void BookmarkMgr::updateActiveList() { - std::lock_guard < std::recursive_mutex > lockData(busy_lock); - if (wxGetApp().isShuttingDown()) { return; } @@ -430,8 +447,6 @@ void BookmarkMgr::updateActiveList() { void BookmarkMgr::updateBookmarks() { - std::lock_guard < std::recursive_mutex > lockData(busy_lock); - BookmarkView *bmv = wxGetApp().getAppFrame()->getBookmarkView(); if (bmv) { @@ -441,8 +456,6 @@ void BookmarkMgr::updateBookmarks() { void BookmarkMgr::updateBookmarks(std::string group) { - std::lock_guard < std::recursive_mutex > lockData(busy_lock); - BookmarkView *bmv = wxGetApp().getAppFrame()->getBookmarkView(); if (bmv) { @@ -480,9 +493,10 @@ void BookmarkMgr::removeRecent(BookmarkEntryPtr be) { } -const BookmarkList& BookmarkMgr::getRecents() { - std::lock_guard < std::recursive_mutex > lockData(busy_lock); - return recents; +BookmarkList BookmarkMgr::getRecents() { + std::lock_guard < std::recursive_mutex > lock(busy_lock); + //return a copy + return recents; } @@ -508,8 +522,6 @@ void BookmarkMgr::addRange(BookmarkRangeEntryPtr re) { rangesSorted = false; } - - void BookmarkMgr::removeRange(BookmarkRangeEntryPtr re) { std::lock_guard < std::recursive_mutex > lock(busy_lock); @@ -522,7 +534,7 @@ void BookmarkMgr::removeRange(BookmarkRangeEntryPtr re) { } -const BookmarkRangeList& BookmarkMgr::getRanges() { +BookmarkRangeList BookmarkMgr::getRanges() { std::lock_guard < std::recursive_mutex > lock(busy_lock); if (!rangesSorted) { @@ -610,8 +622,6 @@ std::wstring BookmarkMgr::getActiveDisplayName(DemodulatorInstancePtr demod) { void BookmarkMgr::removeActive(DemodulatorInstancePtr demod) { - std::lock_guard < std::recursive_mutex > lock(busy_lock); - if (demod == nullptr) { return; } diff --git a/src/BookmarkMgr.h b/src/BookmarkMgr.h index 0560a67..3d2183f 100644 --- a/src/BookmarkMgr.h +++ b/src/BookmarkMgr.h @@ -16,8 +16,7 @@ class DataNode; class BookmarkEntry { public: - std::mutex busy_lock; - + std::string type; //maps on the Demod user label. std::wstring label; @@ -38,9 +37,7 @@ public: } BookmarkRangeEntry(std::wstring label, long long freq, long long startFreq, long long endFreq) : label(label), freq(freq), startFreq(startFreq), endFreq(endFreq) { } - - std::mutex busy_lock; - + std::wstring label; long long freq; @@ -97,7 +94,9 @@ public: void addGroup(std::string group); void removeGroup(std::string group); void renameGroup(std::string group, std::string ngroup); - const BookmarkList& getBookmarks(std::string group); + //return an independent copy on purpose + BookmarkList getBookmarks(std::string group); + void getGroups(BookmarkNames &arr); void getGroups(wxArrayString &arr); @@ -111,22 +110,29 @@ public: void addRecent(DemodulatorInstancePtr demod); void addRecent(BookmarkEntryPtr be); void removeRecent(BookmarkEntryPtr be); - const BookmarkList& getRecents(); + + //return an independent copy on purpose + BookmarkList getRecents(); + void clearRecents(); void removeActive(DemodulatorInstancePtr demod); void addRange(BookmarkRangeEntryPtr re); void removeRange(BookmarkRangeEntryPtr re); - const BookmarkRangeList& getRanges(); - void clearRanges(); - + + //return an independent copy on purpose + BookmarkRangeList getRanges(); + + void clearRanges(); + static std::wstring getBookmarkEntryDisplayName(BookmarkEntryPtr bmEnt); static std::wstring getActiveDisplayName(DemodulatorInstancePtr demod); protected: void trimRecents(); + void loadDefaultRanges(); BookmarkEntryPtr demodToBookmarkEntry(DemodulatorInstancePtr demod); BookmarkEntryPtr nodeToBookmark(DataNode *node); @@ -136,10 +142,8 @@ protected: BookmarkList recents; BookmarkRangeList ranges; bool rangesSorted; + std::recursive_mutex busy_lock; BookmarkExpandState expandState; - - //represents an empty BookMarkList that is returned by reference by some functions. - static const BookmarkList emptyResults; }; diff --git a/src/forms/Bookmark/BookmarkView.cpp b/src/forms/Bookmark/BookmarkView.cpp index ee84b13..cfe73aa 100644 --- a/src/forms/Bookmark/BookmarkView.cpp +++ b/src/forms/Bookmark/BookmarkView.cpp @@ -301,7 +301,7 @@ wxTreeItemId BookmarkView::refreshBookmarks() { bool groupExpanded = searchState || wxGetApp().getBookmarkMgr().getExpandState(gn_i); - const BookmarkList& bmList = wxGetApp().getBookmarkMgr().getBookmarks(gn_i); + BookmarkList bmList = wxGetApp().getBookmarkMgr().getBookmarks(gn_i); for (auto &bmEnt : bmList) { std::wstring labelVal = BookmarkMgr::getBookmarkEntryDisplayName(bmEnt); @@ -409,7 +409,7 @@ void BookmarkView::doUpdateActiveList() { bool rangeExpandState = searchState?false:expandState["range"]; //Ranges - const BookmarkRangeList& bmRanges = wxGetApp().getBookmarkMgr().getRanges(); + BookmarkRangeList bmRanges = wxGetApp().getBookmarkMgr().getRanges(); m_treeView->DeleteChildren(rangeBranch); @@ -440,8 +440,9 @@ void BookmarkView::doUpdateActiveList() { bool recentExpandState = searchState || expandState["recent"]; // Recents - const BookmarkList& bmRecents = wxGetApp().getBookmarkMgr().getRecents(); - m_treeView->DeleteChildren(recentBranch); + BookmarkList bmRecents = wxGetApp().getBookmarkMgr().getRecents(); + + m_treeView->DeleteChildren(recentBranch); for (auto &bmr_i: bmRecents) { TreeViewItem* tvi = new TreeViewItem(); @@ -1555,21 +1556,6 @@ void BookmarkView::onClearSearch( wxCommandEvent& /* event */ ) { refreshLayout(); } -void BookmarkView::loadDefaultRanges() { - - wxGetApp().getBookmarkMgr().addRange(std::make_shared(L"160 Meters", 1900000, 1800000, 2000000)); - wxGetApp().getBookmarkMgr().addRange(std::make_shared(L"80 Meters", 3750000, 3500000, 4000000)); - wxGetApp().getBookmarkMgr().addRange(std::make_shared(L"60 Meters", 5368500, 5332000, 5405000)); - wxGetApp().getBookmarkMgr().addRange(std::make_shared(L"40 Meters", 7150000, 7000000, 7300000)); - wxGetApp().getBookmarkMgr().addRange(std::make_shared(L"30 Meters", 10125000, 10100000, 10150000)); - wxGetApp().getBookmarkMgr().addRange(std::make_shared(L"20 Meters", 14175000, 14000000, 14350000)); - wxGetApp().getBookmarkMgr().addRange(std::make_shared(L"17 Meters", 18068180, 17044180, 19092180)); - wxGetApp().getBookmarkMgr().addRange(std::make_shared(L"15 Meters", 21225000, 21000000, 21450000)); - wxGetApp().getBookmarkMgr().addRange(std::make_shared(L"12 Meters", 24940000, 24890000, 24990000)); - wxGetApp().getBookmarkMgr().addRange(std::make_shared(L"10 Meters", 28850000, 28000000, 29700000)); -} - - BookmarkRangeEntryPtr BookmarkView::makeActiveRangeEntry() { BookmarkRangeEntryPtr re(new BookmarkRangeEntry); diff --git a/src/forms/Bookmark/BookmarkView.h b/src/forms/Bookmark/BookmarkView.h index 820ece1..673012f 100644 --- a/src/forms/Bookmark/BookmarkView.h +++ b/src/forms/Bookmark/BookmarkView.h @@ -80,7 +80,6 @@ public: bool getExpandState(std::string branchName); void setExpandState(std::string branchName, bool state); - void loadDefaultRanges(); static BookmarkRangeEntryPtr makeActiveRangeEntry(); protected: