Introduce a generic container that provides a thread-safe access to any object by using a mutex which is acquired every time the object accessed.
For example:
Synced<std::unordered_map<int, int>> m{{3, 9}, {4, 16}};
{
SYNCED_LOCK(m, m_locked);
// m_locked represents the internal object, i.e. std::unordered_map,
// while m_locked is in scope the internal mutex is locked.
auto it = m_locked->find(3);
if (it != m_locked->end()) {
std::cout << it->second << std::endl;
}
for (auto& [k, v] : *m_locked) {
std::cout << k << ", " << v << std::endl;
}
}
WITH_SYNCED_LOCK(m, p, p->emplace(5, 25));
Remove the global mutexes g_maplocalhost_mutex, g_deadline_timers_mutex, cs_dir_locks, g_loading_wallet_mutex, g_wallet_release_mutex and use Synced<T> instead.
Benefits
copied from a comment below:
The Synced<T> abstraction is similar to what is suggested in this comment but it does so in a generic way to avoid code repetition. Its benefit is:
- It avoids code repetition at the implementation sites. See PR#26151 for a live example. Namely this:
<details> <summary>Lots of repetitions (92 lines)</summary>
class Foo
{
public:
void PushBack(x)
{
LOCK(m_mutex);
m_data.push_back(x);
}
size_t Size()
{
LOCK(m_mutex);
return m_data.size();
}
// maybe also other methods if needed...
auto Lock()
{
return DebugLock<Mutex>{m_mutex, "Foo::m_mutex", __FILE__, __LINE__};
}
private:
Mutex m_mutex;
std::vector<int> m_data;
};
class Bar
{
public:
void PushBack(x)
{
LOCK(m_mutex);
m_data.push_back(x);
}
size_t Size()
{
LOCK(m_mutex);
return m_data.size();
}
// maybe also other methods if needed...
auto Lock()
{
return DebugLock<Mutex>{m_mutex, "Bar::m_mutex", __FILE__, __LINE__};
}
private:
Mutex m_mutex;
std::vector<std::string> m_data;
};
class Baz
{
public:
void Insert(x)
{
LOCK(m_mutex);
m_data.insert(x);
}
size_t Size()
{
LOCK(m_mutex);
return m_data.size();
}
// maybe also other methods if needed...
auto Lock()
{
return DebugLock<Mutex>{m_mutex, "Baz::m_mutex", __FILE__, __LINE__};
}
private:
Mutex m_mutex;
std::set<std::string> m_data;
};
</details>
becomes this:
<details> <summary>Short (3 lines)</summary>
Synced<std::vector<int>> Foo;
Synced<std::vector<std::string>> Bar;
Synced<std::set<std::string>> Baz;
</details>
- The mutex is properly encapsulated. With a global mutex and a global variable annotated with
GUARDED_BY()it is indeed not possible to add new code that accesses the variable without protection (if using Clang and-Wthread-safety-analysisand-Werror), but it is possible to abuse the mutex and start using it to protect some more, possibly unrelated stuff (we already have this in the current code).
