[Lldb-commits] [lldb] 74b56c7 - Revert "[lldb] Improve locking in PathMappingLists (NFC) (#114576)"
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Sun Nov 3 12:35:19 PST 2024
Author: Jonas Devlieghere
Date: 2024-11-03T12:34:52-08:00
New Revision: 74b56c7eb807e2ba54bd7a2bcfda5d0bceff1c0c
URL: https://github.com/llvm/llvm-project/commit/74b56c7eb807e2ba54bd7a2bcfda5d0bceff1c0c
DIFF: https://github.com/llvm/llvm-project/commit/74b56c7eb807e2ba54bd7a2bcfda5d0bceff1c0c.diff
LOG: Revert "[lldb] Improve locking in PathMappingLists (NFC) (#114576)"
This reverts commit c8209943faeead43c6932c5ddcc69c9e9d1e4262.
Added:
Modified:
lldb/include/lldb/Target/PathMappingList.h
lldb/source/Target/PathMappingList.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/PathMappingList.h b/lldb/include/lldb/Target/PathMappingList.h
index a59539eb0e4bc64..1c0ff564739c5ae 100644
--- a/lldb/include/lldb/Target/PathMappingList.h
+++ b/lldb/include/lldb/Target/PathMappingList.h
@@ -25,6 +25,7 @@ class PathMappingList {
typedef void (*ChangedCallback)(const PathMappingList &path_list,
void *baton);
+ // Constructors and Destructors
PathMappingList();
PathMappingList(ChangedCallback callback, void *callback_baton);
@@ -52,12 +53,12 @@ class PathMappingList {
llvm::json::Value ToJSON();
bool IsEmpty() const {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
return m_pairs.empty();
}
size_t GetSize() const {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
return m_pairs.size();
}
@@ -136,33 +137,25 @@ class PathMappingList {
uint32_t FindIndexForPath(llvm::StringRef path) const;
uint32_t GetModificationID() const {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
return m_mod_id;
}
protected:
+ mutable std::recursive_mutex m_mutex;
typedef std::pair<ConstString, ConstString> pair;
typedef std::vector<pair> 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;
- 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;
+ uint32_t m_mod_id = 0; // Incremented anytime anything is added or removed.
};
} // namespace lldb_private
diff --git a/lldb/source/Target/PathMappingList.cpp b/lldb/source/Target/PathMappingList.cpp
index 81d10bdd53f9207..9c283b0146fe07d 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -48,10 +48,7 @@ PathMappingList::PathMappingList(const PathMappingList &rhs)
const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
if (this != &rhs) {
- std::scoped_lock<std::mutex, std::mutex> callback_locks(
- m_callback_mutex, rhs.m_callback_mutex);
- std::scoped_lock<std::mutex, std::mutex> pairs_locks(m_pairs_mutex,
- rhs.m_pairs_mutex);
+ std::scoped_lock<std::recursive_mutex, std::recursive_mutex> locks(m_mutex, rhs.m_mutex);
m_pairs = rhs.m_pairs;
m_callback = nullptr;
m_callback_baton = nullptr;
@@ -62,105 +59,85 @@ const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
PathMappingList::~PathMappingList() = default;
-void PathMappingList::AppendImpl(llvm::StringRef path,
- llvm::StringRef replacement) {
+void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
+ bool notify) {
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
++m_mod_id;
m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
-}
-
-void PathMappingList::Notify(bool notify) const {
- std::lock_guard<std::mutex> 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<std::mutex> lock(m_pairs_mutex);
- AppendImpl(path, replacement);
- }
- Notify(notify);
-}
-
void PathMappingList::Append(const PathMappingList &rhs, bool notify) {
- {
- std::scoped_lock<std::mutex, std::mutex> locks(m_pairs_mutex,
- rhs.m_pairs_mutex);
- ++m_mod_id;
- if (rhs.m_pairs.empty())
- return;
+ std::scoped_lock<std::recursive_mutex, std::recursive_mutex> locks(m_mutex, rhs.m_mutex);
+ ++m_mod_id;
+ if (!rhs.m_pairs.empty()) {
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<std::mutex> 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);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
+ for (const auto &pair : m_pairs) {
+ if (pair.first.GetStringRef() == normalized_path &&
+ pair.second.GetStringRef() == normalized_replacement)
+ return false;
}
- Notify(notify);
+ Append(path, replacement, notify);
return true;
}
void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement,
uint32_t index, bool notify) {
- {
- std::lock_guard<std::mutex> 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);
+ std::lock_guard<std::recursive_mutex> 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);
}
bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef replacement,
uint32_t index, bool notify) {
- {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
- if (index >= m_pairs.size())
- return false;
- ++m_mod_id;
- m_pairs[index] = pair(NormalizePath(path), NormalizePath(replacement));
- }
- Notify(notify);
+ std::lock_guard<std::recursive_mutex> 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);
return true;
}
bool PathMappingList::Remove(size_t index, bool notify) {
- {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
- if (index >= m_pairs.size())
- return false;
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
+ if (index >= m_pairs.size())
+ return false;
- ++m_mod_id;
- iterator iter = m_pairs.begin() + index;
- m_pairs.erase(iter);
- }
- Notify(notify);
+ ++m_mod_id;
+ iterator iter = m_pairs.begin() + index;
+ m_pairs.erase(iter);
+ if (notify && m_callback)
+ m_callback(*this, m_callback_baton);
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<std::mutex> lock(m_pairs_mutex);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
unsigned int numPairs = m_pairs.size();
if (pair_index < 0) {
@@ -178,7 +155,7 @@ void PathMappingList::Dump(Stream *s, int pair_index) {
llvm::json::Value PathMappingList::ToJSON() {
llvm::json::Array entries;
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
for (const auto &pair : m_pairs) {
llvm::json::Array entry{pair.first.GetStringRef().str(),
pair.second.GetStringRef().str()};
@@ -188,13 +165,12 @@ llvm::json::Value PathMappingList::ToJSON() {
}
void PathMappingList::Clear(bool notify) {
- {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
- if (!m_pairs.empty())
- ++m_mod_id;
- m_pairs.clear();
- }
- Notify(notify);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
+ if (!m_pairs.empty())
+ ++m_mod_id;
+ m_pairs.clear();
+ if (notify && m_callback)
+ m_callback(*this, m_callback_baton);
}
bool PathMappingList::RemapPath(ConstString path,
@@ -220,7 +196,7 @@ static void AppendPathComponents(FileSpec &path, llvm::StringRef components,
std::optional<FileSpec> PathMappingList::RemapPath(llvm::StringRef mapping_path,
bool only_if_exists) const {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
if (m_pairs.empty() || mapping_path.empty())
return {};
LazyBool path_is_relative = eLazyBoolCalculate;
@@ -259,7 +235,7 @@ std::optional<llvm::StringRef>
PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const {
std::string path = file.GetPath();
llvm::StringRef path_ref(path);
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
for (const auto &it : m_pairs) {
llvm::StringRef removed_prefix = it.second.GetStringRef();
if (!path_ref.consume_front(it.second.GetStringRef()))
@@ -288,35 +264,34 @@ PathMappingList::FindFile(const FileSpec &orig_spec) const {
bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef new_path,
bool notify) {
- {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
- uint32_t idx = FindIndexForPath(path);
- if (idx >= m_pairs.size())
- return false;
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
+ uint32_t idx = FindIndexForPath(path);
+ if (idx < m_pairs.size()) {
++m_mod_id;
m_pairs[idx].second = ConstString(new_path);
+ if (notify && m_callback)
+ m_callback(*this, m_callback_baton);
+ return true;
}
- Notify(notify);
- return true;
+ return false;
}
bool PathMappingList::Remove(ConstString path, bool notify) {
- {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
- iterator pos = FindIteratorForPath(path);
- if (pos == m_pairs.end())
- return false;
-
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
+ iterator pos = FindIteratorForPath(path);
+ if (pos != m_pairs.end()) {
++m_mod_id;
m_pairs.erase(pos);
+ if (notify && m_callback)
+ m_callback(*this, m_callback_baton);
+ return true;
}
- Notify(notify);
- return true;
+ return false;
}
PathMappingList::const_iterator
PathMappingList::FindIteratorForPath(ConstString path) const {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
const_iterator pos;
const_iterator begin = m_pairs.begin();
const_iterator end = m_pairs.end();
@@ -330,7 +305,7 @@ PathMappingList::FindIteratorForPath(ConstString path) const {
PathMappingList::iterator
PathMappingList::FindIteratorForPath(ConstString path) {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
iterator pos;
iterator begin = m_pairs.begin();
iterator end = m_pairs.end();
@@ -344,7 +319,7 @@ PathMappingList::FindIteratorForPath(ConstString path) {
bool PathMappingList::GetPathsAtIndex(uint32_t idx, ConstString &path,
ConstString &new_path) const {
- std::lock_guard<std::mutex> lock(m_pairs_mutex);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
if (idx < m_pairs.size()) {
path = m_pairs[idx].first;
new_path = m_pairs[idx].second;
@@ -355,7 +330,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<std::mutex> lock(m_pairs_mutex);
+ std::lock_guard<std::recursive_mutex> lock(m_mutex);
const_iterator pos;
const_iterator begin = m_pairs.begin();
const_iterator end = m_pairs.end();
More information about the lldb-commits
mailing list