Skip to content

Commit 12be1f7

Browse files
rmacnak-googleCommit Queue
authored andcommitted
[vm] Test active mutator stealing works with dart:concurrent locks.
- Add Monitor.notifyAll. - Refactor Mutex.runLocked to unlock if interrupted by unwind error. - Fix skipped DLRT_ExitHandleScope during an unwind error. TEST=vm/dart/isolates/many_isolates_blocked_at_monitor_test Bug: #55991 CoreLibraryReviewExempt: VM-only experimental library Change-Id: I7a37544436e22158e45ee44567600fcf7ff7d7b6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409561 Reviewed-by: Alexander Aprelev <[email protected]> Commit-Queue: Ryan Macnak <[email protected]>
1 parent 0e1b8f7 commit 12be1f7

File tree

6 files changed

+90
-37
lines changed

6 files changed

+90
-37
lines changed

runtime/lib/concurrent.cc

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,23 @@ DEFINE_FFI_NATIVE_ENTRY(Mutex_Initialize, void, (Dart_Handle mutex_handle)) {
2525
Dart_NewFinalizableHandle(mutex_handle, mutex, sizeof(Mutex), DeleteMutex);
2626
};
2727

28-
DEFINE_FFI_NATIVE_ENTRY(Mutex_Lock, void, (Dart_Handle mutex_handle)) {
28+
DEFINE_FFI_NATIVE_ENTRY(Mutex_RunLocked,
29+
Dart_Handle,
30+
(Dart_Handle mutex_handle,
31+
Dart_Handle closure_handle)) {
2932
Mutex* mutex;
3033
Dart_Handle result = Dart_GetNativeInstanceField(
3134
mutex_handle, kMutexNativeField, reinterpret_cast<intptr_t*>(&mutex));
3235
if (Dart_IsError(result)) {
3336
Dart_PropagateError(result);
3437
}
3538
mutex->Lock();
36-
}
37-
38-
DEFINE_FFI_NATIVE_ENTRY(Mutex_Unlock, void, (Dart_Handle mutex_handle)) {
39-
Mutex* mutex;
40-
Dart_Handle result = Dart_GetNativeInstanceField(
41-
mutex_handle, kMutexNativeField, reinterpret_cast<intptr_t*>(&mutex));
39+
result = Dart_InvokeClosure(closure_handle, 0, nullptr);
40+
mutex->Unlock();
4241
if (Dart_IsError(result)) {
4342
Dart_PropagateError(result);
4443
}
45-
mutex->Unlock();
44+
return result;
4645
}
4746

4847
static void DeleteConditionVariable(void* isolate_data, void* condvar_pointer) {
@@ -98,4 +97,17 @@ DEFINE_FFI_NATIVE_ENTRY(ConditionVariable_Notify,
9897
condvar->Notify();
9998
}
10099

100+
DEFINE_FFI_NATIVE_ENTRY(ConditionVariable_NotifyAll,
101+
void,
102+
(Dart_Handle condvar_handle)) {
103+
ConditionVariable* condvar;
104+
Dart_Handle result_condvar =
105+
Dart_GetNativeInstanceField(condvar_handle, kCondVarNativeField,
106+
reinterpret_cast<intptr_t*>(&condvar));
107+
if (Dart_IsError(result_condvar)) {
108+
Dart_PropagateError(result_condvar);
109+
}
110+
condvar->NotifyAll();
111+
}
112+
101113
} // namespace dart
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
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.
4+
5+
// VMOptions=--experimental-shared-data
6+
7+
import "dart:concurrent";
8+
import "dart:isolate";
9+
import "dart:io";
10+
11+
@pragma("vm:shared")
12+
int pending = 0;
13+
@pragma("vm:shared")
14+
late Mutex mutex;
15+
@pragma("vm:shared")
16+
late ConditionVariable condition;
17+
18+
waitForAllToCheckIn() {
19+
mutex.runLocked(() {
20+
pending--;
21+
if (pending == 0) {
22+
print("notifyAll $pending");
23+
condition.notifyAll();
24+
} else {
25+
while (pending > 0) {
26+
print("wait $pending");
27+
condition.wait(mutex);
28+
print("notified $pending");
29+
}
30+
}
31+
condition.notifyAll();
32+
});
33+
}
34+
35+
child(_) {
36+
waitForAllToCheckIn();
37+
}
38+
39+
main() async {
40+
mutex = new Mutex();
41+
condition = new ConditionVariable();
42+
pending = 21;
43+
for (var i = 0; i < 20; i++) {
44+
Isolate.spawn(child, null);
45+
}
46+
waitForAllToCheckIn();
47+
}

runtime/vm/bootstrap_natives.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,11 +368,11 @@ namespace dart {
368368
#define BOOTSTRAP_FFI_NATIVE_LIST(V) \
369369
V(ConditionVariable_Initialize, void, (Dart_Handle)) \
370370
V(ConditionVariable_Notify, void, (Dart_Handle)) \
371+
V(ConditionVariable_NotifyAll, void, (Dart_Handle)) \
371372
V(ConditionVariable_Wait, void, (Dart_Handle, Dart_Handle)) \
372373
V(FinalizerEntry_SetExternalSize, void, (Dart_Handle, intptr_t)) \
373374
V(Mutex_Initialize, void, (Dart_Handle)) \
374-
V(Mutex_Lock, void, (Dart_Handle)) \
375-
V(Mutex_Unlock, void, (Dart_Handle)) \
375+
V(Mutex_RunLocked, Dart_Handle, (Dart_Handle, Dart_Handle)) \
376376
V(Pointer_asTypedListFinalizerAllocateData, void*, ()) \
377377
V(Pointer_asTypedListFinalizerCallbackPointer, void*, ())
378378

runtime/vm/dart_entry.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,18 @@ class DartEntryScope : public TransitionToGenerated {
5252
saved_safestack_limit_ = OSThread::GetCurrentSafestackPointer();
5353
thread->set_saved_safestack_limit(saved_safestack_limit_);
5454
#endif
55+
56+
saved_api_scope_ = thread->api_top_scope();
5557
}
5658

5759
~DartEntryScope() {
60+
// Propagating an Error that is not an UnhandledException, such as an
61+
// UnwindError, will bypass the DLRT_ExitHandleScope in an FFI callout
62+
// trampoline.
63+
while (UNLIKELY(thread()->api_top_scope() != saved_api_scope_)) {
64+
thread()->ExitApiScope();
65+
}
66+
5867
#if defined(USING_SAFE_STACK)
5968
thread()->set_saved_safestack_limit(saved_safestack_limit_);
6069
#endif
@@ -68,6 +77,7 @@ class DartEntryScope : public TransitionToGenerated {
6877
#if defined(USING_SAFE_STACK)
6978
uword saved_safestack_limit_ = 0;
7079
#endif
80+
ApiLocalScope* saved_api_scope_;
7181
};
7282

7383
// Note: The invocation stub follows the C ABI, so we cannot pass C++ struct

sdk/lib/_internal/vm/lib/concurrent_patch.dart

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,11 @@ base class _MutexImpl extends NativeFieldWrapperClass1 implements Mutex {
2323
external void _initialize();
2424

2525
@patch
26-
@Native<Void Function(Handle)>(symbol: "Mutex_Lock")
27-
external void _lock();
28-
29-
@patch
30-
@Native<Void Function(Handle)>(symbol: "Mutex_Unlock")
31-
external void _unlock();
26+
@Native<Handle Function(Handle, Handle)>(symbol: "Mutex_RunLocked")
27+
external Object _runLocked(Object action);
3228

3329
R runLocked<R>(R Function() action) {
34-
_lock();
35-
try {
36-
return action();
37-
} finally {
38-
_unlock();
39-
}
30+
return _runLocked(action) as R;
4031
}
4132
}
4233

@@ -64,4 +55,8 @@ base class _ConditionVariableImpl extends NativeFieldWrapperClass1
6455
@patch
6556
@Native<Void Function(Handle)>(symbol: "ConditionVariable_Notify")
6657
external void notify();
58+
59+
@patch
60+
@Native<Void Function(Handle)>(symbol: "ConditionVariable_NotifyAll")
61+
external void notifyAll();
6762
}

sdk/lib/concurrent/concurrent.dart

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,7 @@ abstract interface class Mutex {
1919

2020
external factory Mutex._();
2121

22-
/// Acquire exclusive ownership of this mutex.
23-
///
24-
/// If this mutex is already acquired then an attempt to acquire it
25-
/// blocks the current thread until the mutex is released by the
26-
/// current owner.
27-
///
28-
/// **Warning**: attempting to hold a mutex across asynchronous suspension
29-
/// points will lead to undefined behavior and potentially crashes.
30-
external void _lock();
31-
32-
/// Release exclusive ownership of this mutex.
33-
///
34-
/// It is an error to release ownership of the mutex if it was not
35-
/// previously acquired.
36-
external void _unlock();
22+
external Object _runLocked(Object action);
3723

3824
/// Run the given synchronous `action` under a mutex.
3925
///
@@ -64,4 +50,7 @@ abstract interface class ConditionVariable {
6450

6551
/// Wake up at least one thread waiting on this condition variable.
6652
external void notify();
53+
54+
/// Wake up all threads waiting on this condition variable.
55+
external void notifyAll();
6756
}

0 commit comments

Comments
 (0)