Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(414)

Side by Side Diff: runtime/vm/lockers.h

Issue 1748953003: - Add assertions in MutexLocker/MonitorLocker to ensure that the code enclosed (Closed) Base URL: git@github.com:dart-lang/sdk.git@master
Patch Set: self-review-comments Created 4 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file 1 // Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file
2 // for details. All rights reserved. Use of this source code is governed by a 2 // for details. All rights reserved. Use of this source code is governed by a
3 // BSD-style license that can be found in the LICENSE file. 3 // BSD-style license that can be found in the LICENSE file.
4 4
5 #ifndef VM_LOCKERS_H_ 5 #ifndef VM_LOCKERS_H_
6 #define VM_LOCKERS_H_ 6 #define VM_LOCKERS_H_
7 7
8 #include "platform/assert.h" 8 #include "platform/assert.h"
9 #include "vm/allocation.h" 9 #include "vm/allocation.h"
10 #include "vm/globals.h" 10 #include "vm/globals.h"
11 #include "vm/isolate.h" 11 #include "vm/isolate.h"
12 #include "vm/os_thread.h" 12 #include "vm/os_thread.h"
13 13
14 namespace dart { 14 namespace dart {
15 15
16 /*
17 * Normal mutex locker :
18 * This locker abstraction should only be used when the enclosing code will
srdjan 2016/03/01 16:46:23 s/will not/cannot/
siva 2016/03/01 18:59:54 Done.
19 * not trigger a safepoint. In debug mode this class increments the
20 * no_safepoint_scope_depth variable for the current thread when the lock is
21 * taken and decrements it when the lock is released. NOTE: please do not use
22 * the passed in mutex object independent of the locker class, For example the
23 * code below will not assert correctly:
24 * {
25 * MutexLocker ml(m);
26 * ....
27 * m->Exit();
28 * ....
29 * m->Enter();
Cutch 2016/03/01 15:08:07 Why don't we make the Enter and Exit on the Mutex/
siva 2016/03/01 18:59:53 I tried this but there are a number of places wher
30 * ...
31 * }
32 * Always use the locker object even when the lock needs to be released
33 * temporarily, e.g:
34 * {
35 * MutexLocker ml(m);
36 * ....
37 * ml.Exit();
38 * ....
39 * ml.Enter();
40 * ...
41 * }
42 */
16 class MutexLocker : public ValueObject { 43 class MutexLocker : public ValueObject {
17 public: 44 public:
18 explicit MutexLocker(Mutex* mutex) : mutex_(mutex) { 45 explicit MutexLocker(Mutex* mutex, bool no_safepoint_scope = true)
Cutch 2016/03/01 15:08:07 Can we use an enum rather than a boolean as the se
siva 2016/03/01 18:59:54 An enum here seems like an overkill, we want the n
46 : mutex_(mutex), no_safepoint_scope_(no_safepoint_scope) {
19 ASSERT(mutex != NULL); 47 ASSERT(mutex != NULL);
20 // TODO(iposva): Consider adding a no GC scope here. 48 #if defined(DEBUG)
49 if (no_safepoint_scope_) {
50 Thread* thread = Thread::Current();
51 if (thread != NULL) {
52 thread->IncrementNoSafepointScopeDepth();
53 } else {
54 no_safepoint_scope_ = false;
55 }
56 }
57 #endif
21 mutex_->Lock(); 58 mutex_->Lock();
22 } 59 }
23 60
24 virtual ~MutexLocker() { 61 virtual ~MutexLocker() {
25 mutex_->Unlock(); 62 mutex_->Unlock();
26 // TODO(iposva): Consider decrementing the no GC scope here. 63 #if defined(DEBUG)
64 if (no_safepoint_scope_) {
65 Thread::Current()->DecrementNoSafepointScopeDepth();
66 }
67 #endif
68 }
69
70 void Lock() const {
71 #if defined(DEBUG)
72 if (no_safepoint_scope_) {
73 Thread::Current()->IncrementNoSafepointScopeDepth();
74 }
75 #endif
76 mutex_->Lock();
77 }
78 void Unlock() const {
79 mutex_->Unlock();
80 #if defined(DEBUG)
81 if (no_safepoint_scope_) {
82 Thread::Current()->DecrementNoSafepointScopeDepth();
83 }
84 #endif
27 } 85 }
28 86
29 private: 87 private:
30 Mutex* const mutex_; 88 Mutex* const mutex_;
89 bool no_safepoint_scope_;
31 90
32 DISALLOW_COPY_AND_ASSIGN(MutexLocker); 91 DISALLOW_COPY_AND_ASSIGN(MutexLocker);
33 }; 92 };
34 93
35 94 /*
95 * Normal monitor locker :
96 * This locker abstraction should only be used when the enclosing code will
srdjan 2016/03/01 16:46:23 ditto?
siva 2016/03/01 18:59:53 Done.
97 * not trigger a safepoint. In debug mode this class increments the
98 * no_safepoint_scope_depth variable for the current thread when the lock is
99 * taken and decrements it when the lock is released. NOTE: please do not use
100 * the passed in mutex object independent of the locker class, For example the
101 * code below will not assert correctly:
102 * {
103 * MonitorLocker ml(m);
104 * ....
105 * m->Exit();
106 * ....
107 * m->Enter();
108 * ...
109 * }
110 * Always use the locker object even when the lock needs to be released
111 * temporarily, e.g:
112 * {
113 * MonitorLocker ml(m);
114 * ....
115 * ml.Exit();
116 * ....
117 * ml.Enter();
118 * ...
119 * }
120 */
36 class MonitorLocker : public ValueObject { 121 class MonitorLocker : public ValueObject {
37 public: 122 public:
38 explicit MonitorLocker(Monitor* monitor) : monitor_(monitor) { 123 explicit MonitorLocker(Monitor* monitor, bool no_safepoint_scope = true)
124 : monitor_(monitor), no_safepoint_scope_(no_safepoint_scope) {
39 ASSERT(monitor != NULL); 125 ASSERT(monitor != NULL);
40 // TODO(iposva): Consider adding a no GC scope here. 126 #if defined(DEBUG)
127 if (no_safepoint_scope_) {
128 Thread* thread = Thread::Current();
129 if (thread != NULL) {
130 thread->IncrementNoSafepointScopeDepth();
131 } else {
132 no_safepoint_scope_ = false;
133 }
134 }
135 #endif
41 monitor_->Enter(); 136 monitor_->Enter();
42 } 137 }
43 138
44 virtual ~MonitorLocker() { 139 virtual ~MonitorLocker() {
45 monitor_->Exit(); 140 monitor_->Exit();
46 // TODO(iposva): Consider decrementing the no GC scope here. 141 #if defined(DEBUG)
Cutch 2016/03/01 15:08:07 here and elsewhere: DEBUG_ONLY( )
siva 2016/03/01 18:59:54 I looked at uses of DEBUG_ONLY and looks like the
142 if (no_safepoint_scope_) {
143 Thread::Current()->DecrementNoSafepointScopeDepth();
144 }
145 #endif
146 }
147
148 void Enter() const {
149 #if defined(DEBUG)
150 if (no_safepoint_scope_) {
151 Thread::Current()->IncrementNoSafepointScopeDepth();
152 }
153 #endif
154 monitor_->Enter();
155 }
156 void Exit() const {
157 monitor_->Exit();
158 #if defined(DEBUG)
159 if (no_safepoint_scope_) {
160 Thread::Current()->DecrementNoSafepointScopeDepth();
161 }
162 #endif
47 } 163 }
48 164
49 Monitor::WaitResult Wait(int64_t millis = Monitor::kNoTimeout) { 165 Monitor::WaitResult Wait(int64_t millis = Monitor::kNoTimeout) {
50 return monitor_->Wait(millis); 166 return monitor_->Wait(millis);
51 } 167 }
52 168
53 Monitor::WaitResult WaitWithSafepointCheck( 169 Monitor::WaitResult WaitWithSafepointCheck(
54 Thread* thread, int64_t millis = Monitor::kNoTimeout); 170 Thread* thread, int64_t millis = Monitor::kNoTimeout);
55 171
56 Monitor::WaitResult WaitMicros(int64_t micros = Monitor::kNoTimeout) { 172 Monitor::WaitResult WaitMicros(int64_t micros = Monitor::kNoTimeout) {
57 return monitor_->WaitMicros(micros); 173 return monitor_->WaitMicros(micros);
58 } 174 }
59 175
60 void Notify() { 176 void Notify() {
61 monitor_->Notify(); 177 monitor_->Notify();
62 } 178 }
63 179
64 void NotifyAll() { 180 void NotifyAll() {
65 monitor_->NotifyAll(); 181 monitor_->NotifyAll();
66 } 182 }
67 183
68 private: 184 private:
69 Monitor* const monitor_; 185 Monitor* const monitor_;
186 bool no_safepoint_scope_;
70 187
71 DISALLOW_COPY_AND_ASSIGN(MonitorLocker); 188 DISALLOW_COPY_AND_ASSIGN(MonitorLocker);
72 }; 189 };
73 190
74 191 /*
75 // SafepointMutexLocker objects are used in code where the locks are 192 * Safepoint mutex locker :
76 // more coarse grained and a safepoint operation could be potentially 193 * This locker abstraction should be used when the lock is more coarse
Cutch 2016/03/01 15:08:07 I don't see why the granularity of the lock matter
siva 2016/03/01 18:59:53 Changed the comment to remove the coarse wording.
77 // triggered while holding this lock. This ensures that other threads 194 * grained and the enclosing code could potentially trigger a safepoint.
78 // which try to acquire the same lock will be marked as being at a 195 * This locker ensures that other threads that try to acquire the same lock
79 // safepoint when they are blocked. 196 * will be marked as being at a safepoint if they get blocked trying to
197 * acquire the lock.
198 * NOTE: please do not use the passed in mutex object independent of the locker
199 * class, For example the code below will not work correctly:
200 * {
201 * SafepointMutexLocker ml(m);
202 * ....
203 * m->Exit();
204 * ....
205 * m->Enter();
206 * ...
207 * }
208 */
80 class SafepointMutexLocker : public ValueObject { 209 class SafepointMutexLocker : public ValueObject {
81 public: 210 public:
82 explicit SafepointMutexLocker(Mutex* mutex); 211 explicit SafepointMutexLocker(Mutex* mutex);
83 virtual ~SafepointMutexLocker() { 212 virtual ~SafepointMutexLocker() {
84 mutex_->Unlock(); 213 mutex_->Unlock();
85 } 214 }
86 215
87 private: 216 private:
88 Mutex* const mutex_; 217 Mutex* const mutex_;
89 218
90 DISALLOW_COPY_AND_ASSIGN(SafepointMutexLocker); 219 DISALLOW_COPY_AND_ASSIGN(SafepointMutexLocker);
91 }; 220 };
92 221
222 /*
223 * Safepoint monitor locker :
224 * This locker abstraction should be used when the lock is more coarse
225 * grained and the enclosing code could potentially trigger a safepoint.
226 * This locker ensures that other threads that try to acquire the same lock
227 * will be marked as being at a safepoint if they get blocked trying to
228 * acquire the lock.
229 * NOTE: please do not use the passed in monitor object independent of the locke r
Cutch 2016/03/01 15:08:07 line length
siva 2016/03/01 18:59:53 Done.
230 * class, For example the code below will not work correctly:
231 * {
232 * SafepointMonitorLocker ml(m);
233 * ....
234 * m->Exit();
235 * ....
236 * m->Enter();
237 * ...
238 * }
239 */
240 class SafepointMonitorLocker : public ValueObject {
241 public:
242 explicit SafepointMonitorLocker(Monitor* monitor);
243 virtual ~SafepointMonitorLocker() {
244 monitor_->Exit();
245 }
246
247 Monitor::WaitResult Wait(int64_t millis = Monitor::kNoTimeout);
248
249 private:
250 Monitor* const monitor_;
251
252 DISALLOW_COPY_AND_ASSIGN(SafepointMonitorLocker);
253 };
254
93 } // namespace dart 255 } // namespace dart
94 256
95 257
96 #endif // VM_LOCKERS_H_ 258 #endif // VM_LOCKERS_H_
OLDNEW
« no previous file with comments | « runtime/vm/isolate.cc ('k') | runtime/vm/lockers.cc » ('j') | runtime/vm/lockers.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698