Skip to content

Commit 74846b9

Browse files
aamCommit Queue
authored andcommitted
[vm] Introduce ReentrantMonitor.
Existing Monitor is not reentrable, doesn't support acquiring it if it is already acquired by this thread. TEST=ReentrantMonitorAllowsReentrance Change-Id: Id74479388a99ca907bca6e379c3a7e08191e841b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/436542 Commit-Queue: Alexander Aprelev <[email protected]> Reviewed-by: Ryan Macnak <[email protected]>
1 parent 69f76f1 commit 74846b9

File tree

4 files changed

+118
-16
lines changed

4 files changed

+118
-16
lines changed

runtime/platform/synchronization.h

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,92 @@ class Monitor {
130130
DISALLOW_COPY_AND_ASSIGN(Monitor);
131131
};
132132

133+
class ReentrantMonitor {
134+
public:
135+
using WaitResult = ConditionVariable::WaitResult;
136+
static constexpr WaitResult kNotified = ConditionVariable::kNotified;
137+
static constexpr WaitResult kTimedOut = ConditionVariable::kTimedOut;
138+
139+
static constexpr int64_t kNoTimeout = ConditionVariable::kNoTimeout;
140+
141+
ReentrantMonitor() {}
142+
~ReentrantMonitor() {}
143+
144+
bool IsOwnedByCurrentThread() const {
145+
return owner_ == platform::GetCurrentThreadId();
146+
}
147+
148+
bool TryEnter() {
149+
bool entered = false;
150+
if (IsOwnedByCurrentThread()) {
151+
entered = true;
152+
} else {
153+
entered = mutex_.TryLock();
154+
if (entered) {
155+
owner_ = platform::GetCurrentThreadId();
156+
}
157+
}
158+
if (entered) {
159+
++depth_;
160+
ASSERT(owner_ == platform::GetCurrentThreadId());
161+
}
162+
return entered;
163+
}
164+
165+
void Enter() {
166+
ASSERT(depth_ >= 0);
167+
if (!IsOwnedByCurrentThread()) {
168+
mutex_.Lock();
169+
owner_ = platform::GetCurrentThreadId();
170+
}
171+
ASSERT(owner_ == platform::GetCurrentThreadId());
172+
++depth_;
173+
}
174+
175+
void Exit() {
176+
ASSERT(depth_ > 0);
177+
ASSERT(owner_ == platform::GetCurrentThreadId());
178+
if (--depth_ == 0) {
179+
owner_ = platform::kInvalidThreadId;
180+
mutex_.Unlock();
181+
}
182+
}
183+
184+
WaitResult Wait(int64_t timeout_millis) {
185+
ASSERT(IsOwnedByCurrentThread() && depth_ == 1);
186+
--depth_;
187+
owner_ = platform::kInvalidThreadId;
188+
WaitResult result = cv_.Wait(&mutex_, timeout_millis);
189+
owner_ = platform::GetCurrentThreadId();
190+
++depth_;
191+
return result;
192+
}
193+
194+
WaitResult WaitMicros(int64_t timeout_micros) {
195+
ASSERT(IsOwnedByCurrentThread() && depth_ == 1);
196+
--depth_;
197+
owner_ = platform::kInvalidThreadId;
198+
WaitResult result = cv_.WaitMicros(&mutex_, timeout_micros);
199+
owner_ = platform::GetCurrentThreadId();
200+
++depth_;
201+
return result;
202+
}
203+
204+
// Notify waiting threads.
205+
void Notify() { cv_.Notify(); }
206+
207+
void NotifyAll() { cv_.NotifyAll(); }
208+
209+
private:
210+
Mutex mutex_; // OS-specific data.
211+
ConditionVariable cv_;
212+
213+
intptr_t depth_ = 0;
214+
platform::ThreadId owner_ = platform::kInvalidThreadId;
215+
216+
DISALLOW_COPY_AND_ASSIGN(ReentrantMonitor);
217+
};
218+
133219
} // namespace dart
134220

135221
#endif // RUNTIME_PLATFORM_SYNCHRONIZATION_H_

runtime/vm/lockers.cc

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ SafepointMutexLocker::SafepointMutexLocker(ThreadState* thread, Mutex* mutex)
5555
}
5656
}
5757

58-
void SafepointMonitorLocker::AcquireLock() {
58+
template <typename M>
59+
void SafepointLocker<M>::AcquireLock() {
5960
ASSERT(monitor_ != nullptr);
6061
if (!monitor_->TryEnter()) {
6162
// We did not get the lock and could potentially block, so transition
@@ -70,14 +71,11 @@ void SafepointMonitorLocker::AcquireLock() {
7071
}
7172
}
7273

73-
void SafepointMonitorLocker::ReleaseLock() {
74-
monitor_->Exit();
75-
}
76-
77-
Monitor::WaitResult SafepointMonitorLocker::Wait(int64_t millis) {
74+
template <typename M>
75+
M::WaitResult SafepointLocker<M>::Wait(int64_t millis) {
7876
Thread* thread = Thread::Current();
7977
if (thread != nullptr) {
80-
Monitor::WaitResult result;
78+
typename M::WaitResult result;
8179
{
8280
TransitionVMToBlocked transition(thread);
8381
result = monitor_->Wait(millis);
@@ -88,6 +86,9 @@ Monitor::WaitResult SafepointMonitorLocker::Wait(int64_t millis) {
8886
}
8987
}
9088

89+
template class SafepointLocker<ReentrantMonitor>;
90+
template class SafepointLocker<Monitor>;
91+
9192
#if defined(DEBUG)
9293
bool SafepointRwLock::IsCurrentThreadReader() {
9394
ThreadId id = OSThread::GetCurrentThreadId();

runtime/vm/lockers.h

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -273,28 +273,30 @@ class SafepointMutexLocker : public StackResource {
273273
* ...
274274
* }
275275
*/
276-
class SafepointMonitorLocker : public ValueObject {
276+
template <typename M>
277+
class SafepointLocker : public ValueObject {
277278
public:
278-
explicit SafepointMonitorLocker(Monitor* monitor) : monitor_(monitor) {
279-
AcquireLock();
280-
}
281-
virtual ~SafepointMonitorLocker() { ReleaseLock(); }
279+
explicit SafepointLocker(M* monitor) : monitor_(monitor) { AcquireLock(); }
280+
virtual ~SafepointLocker() { ReleaseLock(); }
282281

283-
Monitor::WaitResult Wait(int64_t millis = Monitor::kNoTimeout);
282+
M::WaitResult Wait(int64_t millis = Monitor::kNoTimeout);
284283

285284
void NotifyAll() { monitor_->NotifyAll(); }
286285

287286
private:
288287
friend class SafepointMonitorUnlockScope;
289288

290289
void AcquireLock();
291-
void ReleaseLock();
292290

293-
Monitor* const monitor_;
291+
void ReleaseLock() { monitor_->Exit(); }
294292

295-
DISALLOW_COPY_AND_ASSIGN(SafepointMonitorLocker);
293+
M* const monitor_;
294+
295+
DISALLOW_COPY_AND_ASSIGN(SafepointLocker);
296296
};
297297

298+
typedef SafepointLocker<Monitor> SafepointMonitorLocker;
299+
298300
class SafepointMonitorUnlockScope : public ValueObject {
299301
public:
300302
explicit SafepointMonitorUnlockScope(SafepointMonitorLocker* locker)

runtime/vm/thread_test.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,4 +1154,17 @@ ISOLATE_UNIT_TEST_CASE(SafepointMonitorUnlockScope) {
11541154
}
11551155
}
11561156

1157+
ISOLATE_UNIT_TEST_CASE(ReentrantMonitorAllowsReentrance) {
1158+
ReentrantMonitor monitor;
1159+
{
1160+
SafepointLocker<ReentrantMonitor> ml1(&monitor);
1161+
ASSERT(monitor.IsOwnedByCurrentThread());
1162+
{
1163+
SafepointLocker<ReentrantMonitor> ml2(&monitor);
1164+
ASSERT(monitor.IsOwnedByCurrentThread());
1165+
}
1166+
ASSERT(monitor.IsOwnedByCurrentThread());
1167+
}
1168+
}
1169+
11571170
} // namespace dart

0 commit comments

Comments
 (0)