diff --git a/lldb/include/lldb/Target/PathMappingList.h b/lldb/include/lldb/Target/PathMappingList.h index 1c0ff564739c5..a59539eb0e4bc 100644 --- a/lldb/include/lldb/Target/PathMappingList.h +++ b/lldb/include/lldb/Target/PathMappingList.h @@ -25,7 +25,6 @@ class PathMappingList { typedef void (*ChangedCallback)(const PathMappingList &path_list, void *baton); - // Constructors and Destructors PathMappingList(); PathMappingList(ChangedCallback callback, void *callback_baton); @@ -53,12 +52,12 @@ class PathMappingList { llvm::json::Value ToJSON(); bool IsEmpty() const { - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_pairs_mutex); return m_pairs.empty(); } size_t GetSize() const { - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_pairs_mutex); return m_pairs.size(); } @@ -137,25 +136,33 @@ class PathMappingList { uint32_t FindIndexForPath(llvm::StringRef path) const; uint32_t GetModificationID() const { - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_pairs_mutex); return m_mod_id; } protected: - mutable std::recursive_mutex m_mutex; typedef std::pair pair; typedef std::vector collection; typedef collection::iterator iterator; typedef collection::const_iterator const_iterator; + void AppendImpl(llvm::StringRef path, llvm::StringRef replacement); + void Notify(bool notify) const; + iterator FindIteratorForPath(ConstString path); const_iterator FindIteratorForPath(ConstString path) const; collection m_pairs; + mutable std::mutex m_pairs_mutex; + ChangedCallback m_callback = nullptr; void *m_callback_baton = nullptr; - uint32_t m_mod_id = 0; // Incremented anytime anything is added or removed. + mutable std::mutex m_callback_mutex; + + /// Incremented anytime anything is added to or removed from m_pairs. Also + /// protected by m_pairs_mutex. + uint32_t m_mod_id = 0; }; } // namespace lldb_private diff --git a/lldb/source/Target/PathMappingList.cpp b/lldb/source/Target/PathMappingList.cpp index 9c283b0146fe0..81d10bdd53f92 100644 --- a/lldb/source/Target/PathMappingList.cpp +++ b/lldb/source/Target/PathMappingList.cpp @@ -48,7 +48,10 @@ PathMappingList::PathMappingList(const PathMappingList &rhs) const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) { if (this != &rhs) { - std::scoped_lock locks(m_mutex, rhs.m_mutex); + std::scoped_lock callback_locks( + m_callback_mutex, rhs.m_callback_mutex); + std::scoped_lock pairs_locks(m_pairs_mutex, + rhs.m_pairs_mutex); m_pairs = rhs.m_pairs; m_callback = nullptr; m_callback_baton = nullptr; @@ -59,85 +62,105 @@ const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) { PathMappingList::~PathMappingList() = default; -void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement, - bool notify) { - std::lock_guard lock(m_mutex); +void PathMappingList::AppendImpl(llvm::StringRef path, + llvm::StringRef replacement) { ++m_mod_id; m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement))); +} + +void PathMappingList::Notify(bool notify) const { + std::lock_guard lock(m_callback_mutex); if (notify && m_callback) m_callback(*this, m_callback_baton); } +void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement, + bool notify) { + { + std::lock_guard lock(m_pairs_mutex); + AppendImpl(path, replacement); + } + Notify(notify); +} + void PathMappingList::Append(const PathMappingList &rhs, bool notify) { - std::scoped_lock locks(m_mutex, rhs.m_mutex); - ++m_mod_id; - if (!rhs.m_pairs.empty()) { + { + std::scoped_lock locks(m_pairs_mutex, + rhs.m_pairs_mutex); + ++m_mod_id; + if (rhs.m_pairs.empty()) + return; const_iterator pos, end = rhs.m_pairs.end(); for (pos = rhs.m_pairs.begin(); pos != end; ++pos) m_pairs.push_back(*pos); - if (notify && m_callback) - m_callback(*this, m_callback_baton); } + Notify(notify); } bool PathMappingList::AppendUnique(llvm::StringRef path, llvm::StringRef replacement, bool notify) { auto normalized_path = NormalizePath(path); auto normalized_replacement = NormalizePath(replacement); - std::lock_guard lock(m_mutex); - for (const auto &pair : m_pairs) { - if (pair.first.GetStringRef() == normalized_path && - pair.second.GetStringRef() == normalized_replacement) - return false; + { + std::lock_guard lock(m_pairs_mutex); + for (const auto &pair : m_pairs) { + if (pair.first.GetStringRef() == normalized_path && + pair.second.GetStringRef() == normalized_replacement) + return false; + } + AppendImpl(path, replacement); } - Append(path, replacement, notify); + Notify(notify); return true; } void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement, uint32_t index, bool notify) { - std::lock_guard lock(m_mutex); - ++m_mod_id; - iterator insert_iter; - if (index >= m_pairs.size()) - insert_iter = m_pairs.end(); - else - insert_iter = m_pairs.begin() + index; - m_pairs.emplace(insert_iter, pair(NormalizePath(path), - NormalizePath(replacement))); - if (notify && m_callback) - m_callback(*this, m_callback_baton); + { + std::lock_guard lock(m_pairs_mutex); + ++m_mod_id; + iterator insert_iter; + if (index >= m_pairs.size()) + insert_iter = m_pairs.end(); + else + insert_iter = m_pairs.begin() + index; + m_pairs.emplace(insert_iter, + pair(NormalizePath(path), NormalizePath(replacement))); + } + Notify(notify); } bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef replacement, uint32_t index, bool notify) { - std::lock_guard lock(m_mutex); - if (index >= m_pairs.size()) - return false; - ++m_mod_id; - m_pairs[index] = pair(NormalizePath(path), NormalizePath(replacement)); - if (notify && m_callback) - m_callback(*this, m_callback_baton); + { + std::lock_guard lock(m_pairs_mutex); + if (index >= m_pairs.size()) + return false; + ++m_mod_id; + m_pairs[index] = pair(NormalizePath(path), NormalizePath(replacement)); + } + Notify(notify); return true; } bool PathMappingList::Remove(size_t index, bool notify) { - std::lock_guard lock(m_mutex); - if (index >= m_pairs.size()) - return false; + { + std::lock_guard lock(m_pairs_mutex); + if (index >= m_pairs.size()) + return false; - ++m_mod_id; - iterator iter = m_pairs.begin() + index; - m_pairs.erase(iter); - if (notify && m_callback) - m_callback(*this, m_callback_baton); + ++m_mod_id; + iterator iter = m_pairs.begin() + index; + m_pairs.erase(iter); + } + Notify(notify); return true; } // For clients which do not need the pair index dumped, pass a pair_index >= 0 // to only dump the indicated pair. void PathMappingList::Dump(Stream *s, int pair_index) { - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_pairs_mutex); unsigned int numPairs = m_pairs.size(); if (pair_index < 0) { @@ -155,7 +178,7 @@ void PathMappingList::Dump(Stream *s, int pair_index) { llvm::json::Value PathMappingList::ToJSON() { llvm::json::Array entries; - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_pairs_mutex); for (const auto &pair : m_pairs) { llvm::json::Array entry{pair.first.GetStringRef().str(), pair.second.GetStringRef().str()}; @@ -165,12 +188,13 @@ llvm::json::Value PathMappingList::ToJSON() { } void PathMappingList::Clear(bool notify) { - std::lock_guard lock(m_mutex); - if (!m_pairs.empty()) - ++m_mod_id; - m_pairs.clear(); - if (notify && m_callback) - m_callback(*this, m_callback_baton); + { + std::lock_guard lock(m_pairs_mutex); + if (!m_pairs.empty()) + ++m_mod_id; + m_pairs.clear(); + } + Notify(notify); } bool PathMappingList::RemapPath(ConstString path, @@ -196,7 +220,7 @@ static void AppendPathComponents(FileSpec &path, llvm::StringRef components, std::optional PathMappingList::RemapPath(llvm::StringRef mapping_path, bool only_if_exists) const { - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_pairs_mutex); if (m_pairs.empty() || mapping_path.empty()) return {}; LazyBool path_is_relative = eLazyBoolCalculate; @@ -235,7 +259,7 @@ std::optional PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const { std::string path = file.GetPath(); llvm::StringRef path_ref(path); - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_pairs_mutex); for (const auto &it : m_pairs) { llvm::StringRef removed_prefix = it.second.GetStringRef(); if (!path_ref.consume_front(it.second.GetStringRef())) @@ -264,34 +288,35 @@ PathMappingList::FindFile(const FileSpec &orig_spec) const { bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef new_path, bool notify) { - std::lock_guard lock(m_mutex); - uint32_t idx = FindIndexForPath(path); - if (idx < m_pairs.size()) { + { + std::lock_guard lock(m_pairs_mutex); + uint32_t idx = FindIndexForPath(path); + if (idx >= m_pairs.size()) + return false; ++m_mod_id; m_pairs[idx].second = ConstString(new_path); - if (notify && m_callback) - m_callback(*this, m_callback_baton); - return true; } - return false; + Notify(notify); + return true; } bool PathMappingList::Remove(ConstString path, bool notify) { - std::lock_guard lock(m_mutex); - iterator pos = FindIteratorForPath(path); - if (pos != m_pairs.end()) { + { + std::lock_guard lock(m_pairs_mutex); + iterator pos = FindIteratorForPath(path); + if (pos == m_pairs.end()) + return false; + ++m_mod_id; m_pairs.erase(pos); - if (notify && m_callback) - m_callback(*this, m_callback_baton); - return true; } - return false; + Notify(notify); + return true; } PathMappingList::const_iterator PathMappingList::FindIteratorForPath(ConstString path) const { - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_pairs_mutex); const_iterator pos; const_iterator begin = m_pairs.begin(); const_iterator end = m_pairs.end(); @@ -305,7 +330,7 @@ PathMappingList::FindIteratorForPath(ConstString path) const { PathMappingList::iterator PathMappingList::FindIteratorForPath(ConstString path) { - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_pairs_mutex); iterator pos; iterator begin = m_pairs.begin(); iterator end = m_pairs.end(); @@ -319,7 +344,7 @@ PathMappingList::FindIteratorForPath(ConstString path) { bool PathMappingList::GetPathsAtIndex(uint32_t idx, ConstString &path, ConstString &new_path) const { - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_pairs_mutex); if (idx < m_pairs.size()) { path = m_pairs[idx].first; new_path = m_pairs[idx].second; @@ -330,7 +355,7 @@ bool PathMappingList::GetPathsAtIndex(uint32_t idx, ConstString &path, uint32_t PathMappingList::FindIndexForPath(llvm::StringRef orig_path) const { const ConstString path = ConstString(NormalizePath(orig_path)); - std::lock_guard lock(m_mutex); + std::lock_guard lock(m_pairs_mutex); const_iterator pos; const_iterator begin = m_pairs.begin(); const_iterator end = m_pairs.end();