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:
0Synced<std::unordered_map<int, int>> m{{3, 9}, {4, 16}};
1
2{
3 SYNCED_LOCK(m, m_locked);
4
5 // m_locked represents the internal object, i.e. std::unordered_map,
6 // while m_locked is in scope the internal mutex is locked.
7
8 auto it = m_locked->find(3);
9 if (it != m_locked->end()) {
10 std::cout << it->second << std::endl;
11 }
12
13 for (auto& [k, v] : *m_locked) {
14 std::cout << k << ", " << v << std::endl;
15 }
16}
17
18WITH_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:
0class Foo
1{
2public:
3 void PushBack(x)
4 {
5 LOCK(m_mutex);
6 m_data.push_back(x);
7 }
8
9 size_t Size()
10 {
11 LOCK(m_mutex);
12 return m_data.size();
13 }
14
15 // maybe also other methods if needed...
16
17 auto Lock()
18 {
19 return DebugLock<Mutex>{m_mutex, "Foo::m_mutex", __FILE__, __LINE__};
20 }
21
22private:
23 Mutex m_mutex;
24 std::vector<int> m_data;
25};
26
27class Bar
28{
29public:
30 void PushBack(x)
31 {
32 LOCK(m_mutex);
33 m_data.push_back(x);
34 }
35
36 size_t Size()
37 {
38 LOCK(m_mutex);
39 return m_data.size();
40 }
41
42 // maybe also other methods if needed...
43
44 auto Lock()
45 {
46 return DebugLock<Mutex>{m_mutex, "Bar::m_mutex", __FILE__, __LINE__};
47 }
48
49private:
50 Mutex m_mutex;
51 std::vector<std::string> m_data;
52};
53
54class Baz
55{
56public:
57 void Insert(x)
58 {
59 LOCK(m_mutex);
60 m_data.insert(x);
61 }
62
63 size_t Size()
64 {
65 LOCK(m_mutex);
66 return m_data.size();
67 }
68
69 // maybe also other methods if needed...
70
71 auto Lock()
72 {
73 return DebugLock<Mutex>{m_mutex, "Baz::m_mutex", __FILE__, __LINE__};
74 }
75
76private:
77 Mutex m_mutex;
78 std::set<std::string> m_data;
79};
becomes this:
0Synced<std::vector<int>> Foo;
1Synced<std::vector<std::string>> Bar;
2Synced<std::set<std::string>> Baz;
- 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).
