Skip to content

Commit cb02b87

Browse files
liamappelbeCommit Queue
liamappelbe
authored and
Commit Queue
committed
[vm] Isolate ownership API
go/dart-isolate-ownership-api Change-Id: Ia778a916de3fecec9f0aa1a5c8bc9fd7dd421267 Bug: dart-lang/native#1908 TEST=runtime/vm/dart_api_impl_test.cc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/407700 Reviewed-by: Martin Kustermann <[email protected]> Commit-Queue: Liam Appelbe <[email protected]>
1 parent b1e10bf commit cb02b87

File tree

11 files changed

+225
-21
lines changed

11 files changed

+225
-21
lines changed

‎runtime/bin/dart_api_win.c‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ typedef Dart_Handle (*Dart_NewSendPortType)(Dart_Port);
137137
typedef Dart_Handle (*Dart_NewSendPortExType)(Dart_PortEx);
138138
typedef Dart_Handle (*Dart_SendPortGetIdType)(Dart_Handle, Dart_Port*);
139139
typedef Dart_Handle (*Dart_SendPortGetIdExType)(Dart_Handle, Dart_PortEx*);
140+
typedef void (*Dart_SetCurrentThreadOwnsIsolateType)();
141+
typedef bool (*Dart_GetCurrentThreadOwnsIsolateType)(Dart_Port);
140142
typedef void (*Dart_EnterScopeType)();
141143
typedef void (*Dart_ExitScopeType)();
142144
typedef uint8_t* (*Dart_ScopeAllocateType)(intptr_t);
@@ -544,6 +546,10 @@ static Dart_NewSendPortType Dart_NewSendPortFn = NULL;
544546
static Dart_NewSendPortExType Dart_NewSendPortExFn = NULL;
545547
static Dart_SendPortGetIdType Dart_SendPortGetIdFn = NULL;
546548
static Dart_SendPortGetIdExType Dart_SendPortGetIdExFn = NULL;
549+
static Dart_SetCurrentThreadOwnsIsolateType Dart_SetCurrentThreadOwnsIsolateFn =
550+
NULL;
551+
static Dart_GetCurrentThreadOwnsIsolateType Dart_GetCurrentThreadOwnsIsolateFn =
552+
NULL;
547553
static Dart_EnterScopeType Dart_EnterScopeFn = NULL;
548554
static Dart_ExitScopeType Dart_ExitScopeFn = NULL;
549555
static Dart_ScopeAllocateType Dart_ScopeAllocateFn = NULL;
@@ -936,6 +942,12 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved) {
936942
(Dart_SendPortGetIdType)GetProcAddress(process, "Dart_SendPortGetId");
937943
Dart_SendPortGetIdExFn = (Dart_SendPortGetIdExType)GetProcAddress(
938944
process, "Dart_SendPortGetIdEx");
945+
Dart_SetCurrentThreadOwnsIsolateFn =
946+
(Dart_SetCurrentThreadOwnsIsolateType)GetProcAddress(
947+
process, "Dart_SetCurrentThreadOwnsIsolate");
948+
Dart_GetCurrentThreadOwnsIsolateFn =
949+
(Dart_GetCurrentThreadOwnsIsolateType)GetProcAddress(
950+
process, "Dart_GetCurrentThreadOwnsIsolate");
939951
Dart_EnterScopeFn =
940952
(Dart_EnterScopeType)GetProcAddress(process, "Dart_EnterScope");
941953
Dart_ExitScopeFn =
@@ -1732,6 +1744,14 @@ Dart_Handle Dart_SendPortGetIdEx(Dart_Handle port, Dart_PortEx* portex_id) {
17321744
return Dart_SendPortGetIdExFn(port, portex_id);
17331745
}
17341746

1747+
void Dart_SetCurrentThreadOwnsIsolate() {
1748+
Dart_SetCurrentThreadOwnsIsolateFn();
1749+
}
1750+
1751+
bool Dart_GetCurrentThreadOwnsIsolate(Dart_Port port) {
1752+
return Dart_GetCurrentThreadOwnsIsolateFn(port);
1753+
}
1754+
17351755
void Dart_EnterScope() {
17361756
Dart_EnterScopeFn();
17371757
}

‎runtime/include/dart_api.h‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1707,8 +1707,6 @@ DART_EXPORT DART_API_WARN_UNUSED_RESULT bool Dart_RunLoopAsync(
17071707
Dart_Port on_exit_port,
17081708
char** error);
17091709

1710-
/* TODO(turnidge): Should this be removed from the public api? */
1711-
17121710
/**
17131711
* Gets the main port id for the current isolate.
17141712
*/
@@ -1779,6 +1777,24 @@ DART_EXPORT Dart_Handle Dart_SendPortGetId(Dart_Handle port,
17791777
*/
17801778
DART_EXPORT Dart_Handle Dart_SendPortGetIdEx(Dart_Handle port,
17811779
Dart_PortEx* portex_id);
1780+
1781+
/**
1782+
* Sets the owner thread of the current isolate to be the current thread.
1783+
*
1784+
* Requires there to be a current isolate, and that the isolate is unowned.
1785+
*/
1786+
DART_EXPORT void Dart_SetCurrentThreadOwnsIsolate(void);
1787+
1788+
/**
1789+
* Returns whether the current thread owns the isolate that owns the given port.
1790+
*
1791+
* The port can be the isolate's main port, or any other port owned by the
1792+
* isolate.
1793+
*
1794+
* \param port_id The port to be checked.
1795+
*/
1796+
DART_EXPORT bool Dart_GetCurrentThreadOwnsIsolate(Dart_Port port);
1797+
17821798
/*
17831799
* ======
17841800
* Scopes

‎runtime/include/dart_api_dl.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,15 @@ typedef void (*Dart_NativeMessageHandler_DL)(Dart_Port_DL dest_port_id,
9696
F(Dart_ExitIsolate, void, (void)) \
9797
F(Dart_EnterIsolate, void, (Dart_Isolate)) \
9898
/* Dart_Port */ \
99+
F(Dart_GetMainPortId, Dart_Port, (void)) \
99100
F(Dart_Post, bool, (Dart_Port_DL port_id, Dart_Handle object)) \
100101
F(Dart_NewSendPort, Dart_Handle, (Dart_Port_DL port_id)) \
101102
F(Dart_NewSendPortEx, Dart_Handle, (Dart_PortEx_DL portex_id)) \
102103
F(Dart_SendPortGetId, Dart_Handle, \
103104
(Dart_Handle port, Dart_Port_DL * port_id)) \
104105
F(Dart_SendPortGetIdEx, Dart_Handle, \
105106
(Dart_Handle port, Dart_PortEx_DL * portex_id)) \
107+
F(Dart_GetCurrentThreadOwnsIsolate, bool, (Dart_Port)) \
106108
/* Scopes */ \
107109
F(Dart_EnterScope, void, (void)) \
108110
F(Dart_ExitScope, void, (void)) \

‎runtime/include/dart_version.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@
1111
// On backwards compatible changes the minor version is increased.
1212
// The versioning covers the symbols exposed in dart_api_dl.h
1313
#define DART_API_DL_MAJOR_VERSION 2
14-
#define DART_API_DL_MINOR_VERSION 5
14+
#define DART_API_DL_MINOR_VERSION 6
1515

1616
#endif /* RUNTIME_INCLUDE_DART_VERSION_H_ */ /* NOLINT */

‎runtime/tests/vm/dart/exported_symbols_test.dart‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ main() {
3535
Platform.isMacOS ? "--extern-only" : "--dynamic",
3636
"--defined-only",
3737
"--format=just-symbols",
38-
Platform.executable
38+
Platform.executable,
3939
]);
4040
if (result.exitCode != 0) {
4141
print("nm failed");
@@ -138,6 +138,7 @@ main() {
138138
"Dart_GetPeer",
139139
"Dart_GetStaticMethodClosure",
140140
"Dart_GetStickyError",
141+
"Dart_GetCurrentThreadOwnsIsolate",
141142
"Dart_GetType",
142143
"Dart_GetTypeOfExternalTypedData",
143144
"Dart_GetTypeOfTypedData",
@@ -236,6 +237,7 @@ main() {
236237
"Dart_NewBoolean",
237238
"Dart_NewByteBuffer",
238239
"Dart_NewCompilationError",
240+
"Dart_NewConcurrentNativePort",
239241
"Dart_NewDouble",
240242
"Dart_NewExternalTypedData",
241243
"Dart_NewExternalTypedDataWithFinalizer",
@@ -247,7 +249,6 @@ main() {
247249
"Dart_NewListOfType",
248250
"Dart_NewListOfTypeFilled",
249251
"Dart_NewNativePort",
250-
"Dart_NewConcurrentNativePort",
251252
"Dart_NewPersistentHandle",
252253
"Dart_NewSendPort",
253254
"Dart_NewSendPortEx",
@@ -315,6 +316,7 @@ main() {
315316
"Dart_SetShouldPauseOnStart",
316317
"Dart_SetStickyError",
317318
"Dart_SetThreadName",
319+
"Dart_SetCurrentThreadOwnsIsolate",
318320
"Dart_SetTimelineRecorderCallback",
319321
"Dart_SetVMFlags",
320322
"Dart_SetWeakHandleReturnValue",
@@ -359,9 +361,7 @@ main() {
359361
"Dart_UnloadELF",
360362
]);
361363
if (!Platform.isMacOS) {
362-
expectedSymbols.addAll([
363-
"Dart_LoadELF_Fd",
364-
]);
364+
expectedSymbols.addAll(["Dart_LoadELF_Fd"]);
365365
}
366366
}
367367

‎runtime/vm/dart_api_impl.cc‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1527,14 +1527,23 @@ DART_EXPORT void Dart_EnterIsolate(Dart_Isolate isolate) {
15271527
CHECK_NO_ISOLATE(Isolate::Current());
15281528
// TODO(http://dartbug.com/16615): Validate isolate parameter.
15291529
Isolate* iso = reinterpret_cast<Isolate*>(isolate);
1530+
ThreadId os_thread = OSThread::GetCurrentThreadId();
15301531
if (iso->IsScheduled()) {
15311532
FATAL(
15321533
"Isolate %s is already scheduled on mutator thread %p, "
15331534
"failed to schedule from os thread 0x%" Px "\n",
15341535
iso->name(), iso->scheduled_mutator_thread(),
1535-
OSThread::ThreadIdToIntPtr(OSThread::GetCurrentThreadId()));
1536+
OSThread::ThreadIdToIntPtr(os_thread));
15361537
}
15371538
Thread::EnterIsolate(iso);
1539+
ThreadId owner_thread = iso->GetOwnerThread(nullptr);
1540+
if (owner_thread != OSThread::kInvalidThreadId && owner_thread != os_thread) {
1541+
FATAL("Isolate %s is owned by os thread 0x%" Px
1542+
", "
1543+
"failed to schedule from os thread 0x%" Px "\n",
1544+
iso->name(), OSThread::ThreadIdToIntPtr(owner_thread),
1545+
OSThread::ThreadIdToIntPtr(os_thread));
1546+
}
15381547
// A Thread structure has been associated to the thread, we do the
15391548
// safepoint transition explicitly here instead of using the
15401549
// TransitionXXX scope objects as the reverse transition happens
@@ -2199,6 +2208,20 @@ DART_EXPORT Dart_Port Dart_GetMainPortId() {
21992208
return isolate->main_port();
22002209
}
22012210

2211+
DART_EXPORT void Dart_SetCurrentThreadOwnsIsolate() {
2212+
Isolate* isolate = Isolate::Current();
2213+
CHECK_ISOLATE(isolate);
2214+
if (!isolate->SetOwnerThread(OSThread::kInvalidThreadId,
2215+
OSThread::GetCurrentThreadId())) {
2216+
FATAL("Tried to claim ownership of isolate %s, but it is already owned\n",
2217+
isolate->name());
2218+
}
2219+
}
2220+
2221+
DART_EXPORT bool Dart_GetCurrentThreadOwnsIsolate(Dart_Port port) {
2222+
return PortMap::IsOwnedByCurrentThread(port);
2223+
}
2224+
22022225
// --- Scopes ----
22032226

22042227
DART_EXPORT void Dart_EnterScope() {

‎runtime/vm/dart_api_impl_test.cc‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
// BSD-style license that can be found in the LICENSE file.
44

55
#include "vm/dart_api_impl.h"
6+
7+
#include <thread> // NOLINT(build/c++11)
8+
69
#include "bin/builtin.h"
710
#include "bin/dartutils.h"
811
#include "include/dart_api.h"
@@ -19,6 +22,7 @@
1922
#include "vm/flags.h"
2023
#include "vm/heap/verifier.h"
2124
#include "vm/lockers.h"
25+
#include "vm/native_message_handler.h"
2226
#include "vm/timeline.h"
2327
#include "vm/unit_test.h"
2428

@@ -213,6 +217,110 @@ TEST_CASE(Dart_KillIsolatePriority) {
213217
EXPECT(interrupted);
214218
}
215219

220+
TEST_CASE(DartAPI_IsolateOwnership) {
221+
Dart_Handle lib = TestCase::LoadTestScript("", nullptr);
222+
EXPECT_VALID(lib);
223+
224+
Dart_Isolate isolate = Dart_CurrentIsolate();
225+
226+
NativeMessageHandler dummy_handler("test", nullptr, 1);
227+
Dart_Port dummy_port = PortMap::CreatePort(&dummy_handler);
228+
229+
Dart_Port port = Dart_GetMainPortId();
230+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port));
231+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port));
232+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT));
233+
234+
Dart_SetCurrentThreadOwnsIsolate();
235+
EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port));
236+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port));
237+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT));
238+
239+
Dart_ExitIsolate();
240+
EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port));
241+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port));
242+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT));
243+
244+
Dart_Port other_port;
245+
std::thread([port, dummy_port, isolate, &other_port]() {
246+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port));
247+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port));
248+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT));
249+
250+
char* error = nullptr;
251+
Dart_CreateIsolateInGroup(isolate, "other", nullptr, nullptr, nullptr,
252+
&error);
253+
EXPECT_EQ(nullptr, error);
254+
other_port = Dart_GetMainPortId();
255+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(other_port));
256+
257+
Dart_SetCurrentThreadOwnsIsolate();
258+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port));
259+
EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(other_port));
260+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port));
261+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT));
262+
263+
Dart_ShutdownIsolate();
264+
}).join();
265+
266+
EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port));
267+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(other_port));
268+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port));
269+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT));
270+
271+
Dart_EnterIsolate(isolate);
272+
EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port));
273+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(other_port));
274+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port));
275+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT));
276+
277+
Dart_ExitIsolate();
278+
PortMap::ClosePort(dummy_port);
279+
Dart_EnterIsolate(isolate);
280+
}
281+
282+
TEST_CASE_WITH_EXPECTATION(DartAPI_IsolateOwnership_SetWhenAlreadyOwned,
283+
"Crash") {
284+
Dart_Handle lib = TestCase::LoadTestScript("", nullptr);
285+
EXPECT_VALID(lib);
286+
287+
Dart_Port port = Dart_GetMainPortId();
288+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port));
289+
290+
Dart_SetCurrentThreadOwnsIsolate();
291+
EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port));
292+
293+
// Causes an assertion failure, because this thread already owns the isolate.
294+
Dart_SetCurrentThreadOwnsIsolate();
295+
}
296+
297+
TEST_CASE_WITH_EXPECTATION(
298+
DartAPI_IsolateOwnership_EnterIsolateOwnedByOtherThread,
299+
"Crash") {
300+
Dart_Handle lib = TestCase::LoadTestScript("", nullptr);
301+
EXPECT_VALID(lib);
302+
303+
Dart_Isolate isolate = Dart_CurrentIsolate();
304+
305+
Dart_Port port = Dart_GetMainPortId();
306+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port));
307+
308+
Dart_SetCurrentThreadOwnsIsolate();
309+
EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port));
310+
311+
Dart_ExitIsolate();
312+
313+
std::thread([isolate, port]() {
314+
EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port));
315+
316+
// Causes an assertion failure, because the isolate is already owned by
317+
// another thread.
318+
Dart_EnterIsolate(isolate);
319+
}).join();
320+
321+
Dart_EnterIsolate(isolate);
322+
}
323+
216324
TEST_CASE(DartAPI_ErrorHandleBasics) {
217325
const char* kScriptChars =
218326
"@pragma('vm:entry-point', 'call')\n"

‎runtime/vm/isolate.cc‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
#include "vm/object_id_ring.h"
4040
#include "vm/object_store.h"
4141
#include "vm/os_thread.h"
42-
#include "vm/port.h"
4342
#include "vm/profiler.h"
4443
#include "vm/reusable_handles.h"
4544
#include "vm/reverse_pc_lookup_cache.h"
@@ -1815,6 +1814,7 @@ Isolate::Isolate(IsolateGroup* isolate_group,
18151814
on_cleanup_callback_(Isolate::CleanupCallback()),
18161815
random_(),
18171816
mutex_(),
1817+
owner_thread_(OSThread::kInvalidThreadId),
18181818
tag_table_(GrowableObjectArray::null()),
18191819
sticky_error_(Error::null()),
18201820
spawn_count_monitor_(),
@@ -2859,6 +2859,15 @@ void Isolate::SetPrefixIsLoaded(const LibraryPrefix& prefix) {
28592859
loaded_prefixes_set_storage_ = loaded_prefixes_set.Release().ptr();
28602860
}
28612861

2862+
bool Isolate::SetOwnerThread(ThreadId expected_old_owner, ThreadId new_owner) {
2863+
return owner_thread_.compare_exchange_strong(expected_old_owner, new_owner);
2864+
}
2865+
2866+
ThreadId Isolate::GetOwnerThread(PortMap::Locker* locker) {
2867+
ASSERT(Isolate::Current() == this || locker != nullptr);
2868+
return owner_thread_.load();
2869+
}
2870+
28622871
void IsolateGroup::EnableIncrementalBarrier(
28632872
MarkingStack* old_marking_stack,
28642873
MarkingStack* new_marking_stack,

‎runtime/vm/isolate.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "vm/megamorphic_cache_table.h"
3030
#include "vm/metrics.h"
3131
#include "vm/os_thread.h"
32+
#include "vm/port.h"
3233
#include "vm/random.h"
3334
#include "vm/service.h"
3435
#include "vm/tags.h"
@@ -1478,6 +1479,12 @@ class Isolate : public IntrusiveDListEntry<Isolate> {
14781479
return &pointers_to_verify_at_exit_;
14791480
}
14801481

1482+
bool SetOwnerThread(ThreadId expected_old_owner, ThreadId new_owner);
1483+
1484+
// Must be invoked with a valid PortMap::Locker, or while this isolate is the
1485+
// current isolate (in which case the locker may be null).
1486+
ThreadId GetOwnerThread(PortMap::Locker* locker);
1487+
14811488
private:
14821489
friend class Dart; // Init, InitOnce, Shutdown.
14831490
friend class IsolateKillerVisitor; // Kill().
@@ -1675,6 +1682,7 @@ class Isolate : public IntrusiveDListEntry<Isolate> {
16751682
DeoptContext* deopt_context_ = nullptr;
16761683
FfiCallbackMetadata::Metadata* ffi_callback_list_head_ = nullptr;
16771684
intptr_t ffi_callback_keep_alive_counter_ = 0;
1685+
RelaxedAtomic<ThreadId> owner_thread_ = OSThread::kInvalidThreadId;
16781686

16791687
GrowableObjectArrayPtr tag_table_;
16801688

0 commit comments

Comments
 (0)