diff --git a/runtime/bin/dart_api_win.c b/runtime/bin/dart_api_win.c index 4bf4e72bd1d..c8924a21904 100644 --- a/runtime/bin/dart_api_win.c +++ b/runtime/bin/dart_api_win.c @@ -137,6 +137,8 @@ typedef Dart_Handle (*Dart_NewSendPortType)(Dart_Port); typedef Dart_Handle (*Dart_NewSendPortExType)(Dart_PortEx); typedef Dart_Handle (*Dart_SendPortGetIdType)(Dart_Handle, Dart_Port*); typedef Dart_Handle (*Dart_SendPortGetIdExType)(Dart_Handle, Dart_PortEx*); +typedef void (*Dart_SetCurrentThreadOwnsIsolateType)(); +typedef bool (*Dart_GetCurrentThreadOwnsIsolateType)(Dart_Port); typedef void (*Dart_EnterScopeType)(); typedef void (*Dart_ExitScopeType)(); typedef uint8_t* (*Dart_ScopeAllocateType)(intptr_t); @@ -544,6 +546,10 @@ static Dart_NewSendPortType Dart_NewSendPortFn = NULL; static Dart_NewSendPortExType Dart_NewSendPortExFn = NULL; static Dart_SendPortGetIdType Dart_SendPortGetIdFn = NULL; static Dart_SendPortGetIdExType Dart_SendPortGetIdExFn = NULL; +static Dart_SetCurrentThreadOwnsIsolateType Dart_SetCurrentThreadOwnsIsolateFn = + NULL; +static Dart_GetCurrentThreadOwnsIsolateType Dart_GetCurrentThreadOwnsIsolateFn = + NULL; static Dart_EnterScopeType Dart_EnterScopeFn = NULL; static Dart_ExitScopeType Dart_ExitScopeFn = NULL; static Dart_ScopeAllocateType Dart_ScopeAllocateFn = NULL; @@ -936,6 +942,12 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved) { (Dart_SendPortGetIdType)GetProcAddress(process, "Dart_SendPortGetId"); Dart_SendPortGetIdExFn = (Dart_SendPortGetIdExType)GetProcAddress( process, "Dart_SendPortGetIdEx"); + Dart_SetCurrentThreadOwnsIsolateFn = + (Dart_SetCurrentThreadOwnsIsolateType)GetProcAddress( + process, "Dart_SetCurrentThreadOwnsIsolate"); + Dart_GetCurrentThreadOwnsIsolateFn = + (Dart_GetCurrentThreadOwnsIsolateType)GetProcAddress( + process, "Dart_GetCurrentThreadOwnsIsolate"); Dart_EnterScopeFn = (Dart_EnterScopeType)GetProcAddress(process, "Dart_EnterScope"); Dart_ExitScopeFn = @@ -1732,6 +1744,14 @@ Dart_Handle Dart_SendPortGetIdEx(Dart_Handle port, Dart_PortEx* portex_id) { return Dart_SendPortGetIdExFn(port, portex_id); } +void Dart_SetCurrentThreadOwnsIsolate() { + Dart_SetCurrentThreadOwnsIsolateFn(); +} + +bool Dart_GetCurrentThreadOwnsIsolate(Dart_Port port) { + return Dart_GetCurrentThreadOwnsIsolateFn(port); +} + void Dart_EnterScope() { Dart_EnterScopeFn(); } diff --git a/runtime/include/dart_api.h b/runtime/include/dart_api.h index 6f73d7fd85e..7394525e0e5 100644 --- a/runtime/include/dart_api.h +++ b/runtime/include/dart_api.h @@ -1707,8 +1707,6 @@ DART_EXPORT DART_API_WARN_UNUSED_RESULT bool Dart_RunLoopAsync( Dart_Port on_exit_port, char** error); -/* TODO(turnidge): Should this be removed from the public api? */ - /** * Gets the main port id for the current isolate. */ @@ -1779,6 +1777,24 @@ DART_EXPORT Dart_Handle Dart_SendPortGetId(Dart_Handle port, */ DART_EXPORT Dart_Handle Dart_SendPortGetIdEx(Dart_Handle port, Dart_PortEx* portex_id); + +/** + * Sets the owner thread of the current isolate to be the current thread. + * + * Requires there to be a current isolate, and that the isolate is unowned. + */ +DART_EXPORT void Dart_SetCurrentThreadOwnsIsolate(void); + +/** + * Returns whether the current thread owns the isolate that owns the given port. + * + * The port can be the isolate's main port, or any other port owned by the + * isolate. + * + * \param port_id The port to be checked. + */ +DART_EXPORT bool Dart_GetCurrentThreadOwnsIsolate(Dart_Port port); + /* * ====== * Scopes diff --git a/runtime/include/dart_api_dl.h b/runtime/include/dart_api_dl.h index 2b4c8d4fdfa..fdaca1a5b58 100644 --- a/runtime/include/dart_api_dl.h +++ b/runtime/include/dart_api_dl.h @@ -96,6 +96,7 @@ typedef void (*Dart_NativeMessageHandler_DL)(Dart_Port_DL dest_port_id, F(Dart_ExitIsolate, void, (void)) \ F(Dart_EnterIsolate, void, (Dart_Isolate)) \ /* Dart_Port */ \ + F(Dart_GetMainPortId, Dart_Port, (void)) \ F(Dart_Post, bool, (Dart_Port_DL port_id, Dart_Handle object)) \ F(Dart_NewSendPort, Dart_Handle, (Dart_Port_DL port_id)) \ F(Dart_NewSendPortEx, Dart_Handle, (Dart_PortEx_DL portex_id)) \ @@ -103,6 +104,7 @@ typedef void (*Dart_NativeMessageHandler_DL)(Dart_Port_DL dest_port_id, (Dart_Handle port, Dart_Port_DL * port_id)) \ F(Dart_SendPortGetIdEx, Dart_Handle, \ (Dart_Handle port, Dart_PortEx_DL * portex_id)) \ + F(Dart_GetCurrentThreadOwnsIsolate, bool, (Dart_Port)) \ /* Scopes */ \ F(Dart_EnterScope, void, (void)) \ F(Dart_ExitScope, void, (void)) \ diff --git a/runtime/include/dart_version.h b/runtime/include/dart_version.h index 5ca0b683c7e..ddfaf3a03fa 100644 --- a/runtime/include/dart_version.h +++ b/runtime/include/dart_version.h @@ -11,6 +11,6 @@ // On backwards compatible changes the minor version is increased. // The versioning covers the symbols exposed in dart_api_dl.h #define DART_API_DL_MAJOR_VERSION 2 -#define DART_API_DL_MINOR_VERSION 5 +#define DART_API_DL_MINOR_VERSION 6 #endif /* RUNTIME_INCLUDE_DART_VERSION_H_ */ /* NOLINT */ diff --git a/runtime/tests/vm/dart/exported_symbols_test.dart b/runtime/tests/vm/dart/exported_symbols_test.dart index 290c1e4fcf8..fd497107f44 100644 --- a/runtime/tests/vm/dart/exported_symbols_test.dart +++ b/runtime/tests/vm/dart/exported_symbols_test.dart @@ -35,7 +35,7 @@ main() { Platform.isMacOS ? "--extern-only" : "--dynamic", "--defined-only", "--format=just-symbols", - Platform.executable + Platform.executable, ]); if (result.exitCode != 0) { print("nm failed"); @@ -138,6 +138,7 @@ main() { "Dart_GetPeer", "Dart_GetStaticMethodClosure", "Dart_GetStickyError", + "Dart_GetCurrentThreadOwnsIsolate", "Dart_GetType", "Dart_GetTypeOfExternalTypedData", "Dart_GetTypeOfTypedData", @@ -236,6 +237,7 @@ main() { "Dart_NewBoolean", "Dart_NewByteBuffer", "Dart_NewCompilationError", + "Dart_NewConcurrentNativePort", "Dart_NewDouble", "Dart_NewExternalTypedData", "Dart_NewExternalTypedDataWithFinalizer", @@ -247,7 +249,6 @@ main() { "Dart_NewListOfType", "Dart_NewListOfTypeFilled", "Dart_NewNativePort", - "Dart_NewConcurrentNativePort", "Dart_NewPersistentHandle", "Dart_NewSendPort", "Dart_NewSendPortEx", @@ -315,6 +316,7 @@ main() { "Dart_SetShouldPauseOnStart", "Dart_SetStickyError", "Dart_SetThreadName", + "Dart_SetCurrentThreadOwnsIsolate", "Dart_SetTimelineRecorderCallback", "Dart_SetVMFlags", "Dart_SetWeakHandleReturnValue", @@ -359,9 +361,7 @@ main() { "Dart_UnloadELF", ]); if (!Platform.isMacOS) { - expectedSymbols.addAll([ - "Dart_LoadELF_Fd", - ]); + expectedSymbols.addAll(["Dart_LoadELF_Fd"]); } } diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index 1e00e011764..4c2fef581c8 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -1527,14 +1527,23 @@ DART_EXPORT void Dart_EnterIsolate(Dart_Isolate isolate) { CHECK_NO_ISOLATE(Isolate::Current()); // TODO(http://dartbug.com/16615): Validate isolate parameter. Isolate* iso = reinterpret_cast(isolate); + ThreadId os_thread = OSThread::GetCurrentThreadId(); if (iso->IsScheduled()) { FATAL( "Isolate %s is already scheduled on mutator thread %p, " "failed to schedule from os thread 0x%" Px "\n", iso->name(), iso->scheduled_mutator_thread(), - OSThread::ThreadIdToIntPtr(OSThread::GetCurrentThreadId())); + OSThread::ThreadIdToIntPtr(os_thread)); } Thread::EnterIsolate(iso); + ThreadId owner_thread = iso->GetOwnerThread(nullptr); + if (owner_thread != OSThread::kInvalidThreadId && owner_thread != os_thread) { + FATAL("Isolate %s is owned by os thread 0x%" Px + ", " + "failed to schedule from os thread 0x%" Px "\n", + iso->name(), OSThread::ThreadIdToIntPtr(owner_thread), + OSThread::ThreadIdToIntPtr(os_thread)); + } // A Thread structure has been associated to the thread, we do the // safepoint transition explicitly here instead of using the // TransitionXXX scope objects as the reverse transition happens @@ -2199,6 +2208,20 @@ DART_EXPORT Dart_Port Dart_GetMainPortId() { return isolate->main_port(); } +DART_EXPORT void Dart_SetCurrentThreadOwnsIsolate() { + Isolate* isolate = Isolate::Current(); + CHECK_ISOLATE(isolate); + if (!isolate->SetOwnerThread(OSThread::kInvalidThreadId, + OSThread::GetCurrentThreadId())) { + FATAL("Tried to claim ownership of isolate %s, but it is already owned\n", + isolate->name()); + } +} + +DART_EXPORT bool Dart_GetCurrentThreadOwnsIsolate(Dart_Port port) { + return PortMap::IsOwnedByCurrentThread(port); +} + // --- Scopes ---- DART_EXPORT void Dart_EnterScope() { diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc index 4f1cf98dac3..7f2206b5539 100644 --- a/runtime/vm/dart_api_impl_test.cc +++ b/runtime/vm/dart_api_impl_test.cc @@ -3,6 +3,9 @@ // BSD-style license that can be found in the LICENSE file. #include "vm/dart_api_impl.h" + +#include // NOLINT(build/c++11) + #include "bin/builtin.h" #include "bin/dartutils.h" #include "include/dart_api.h" @@ -19,6 +22,7 @@ #include "vm/flags.h" #include "vm/heap/verifier.h" #include "vm/lockers.h" +#include "vm/native_message_handler.h" #include "vm/timeline.h" #include "vm/unit_test.h" @@ -213,6 +217,110 @@ TEST_CASE(Dart_KillIsolatePriority) { EXPECT(interrupted); } +TEST_CASE(DartAPI_IsolateOwnership) { + Dart_Handle lib = TestCase::LoadTestScript("", nullptr); + EXPECT_VALID(lib); + + Dart_Isolate isolate = Dart_CurrentIsolate(); + + NativeMessageHandler dummy_handler("test", nullptr, 1); + Dart_Port dummy_port = PortMap::CreatePort(&dummy_handler); + + Dart_Port port = Dart_GetMainPortId(); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT)); + + Dart_SetCurrentThreadOwnsIsolate(); + EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT)); + + Dart_ExitIsolate(); + EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT)); + + Dart_Port other_port; + std::thread([port, dummy_port, isolate, &other_port]() { + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT)); + + char* error = nullptr; + Dart_CreateIsolateInGroup(isolate, "other", nullptr, nullptr, nullptr, + &error); + EXPECT_EQ(nullptr, error); + other_port = Dart_GetMainPortId(); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(other_port)); + + Dart_SetCurrentThreadOwnsIsolate(); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port)); + EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(other_port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT)); + + Dart_ShutdownIsolate(); + }).join(); + + EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(other_port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT)); + + Dart_EnterIsolate(isolate); + EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(other_port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(dummy_port)); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(ILLEGAL_PORT)); + + Dart_ExitIsolate(); + PortMap::ClosePort(dummy_port); + Dart_EnterIsolate(isolate); +} + +TEST_CASE_WITH_EXPECTATION(DartAPI_IsolateOwnership_SetWhenAlreadyOwned, + "Crash") { + Dart_Handle lib = TestCase::LoadTestScript("", nullptr); + EXPECT_VALID(lib); + + Dart_Port port = Dart_GetMainPortId(); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port)); + + Dart_SetCurrentThreadOwnsIsolate(); + EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port)); + + // Causes an assertion failure, because this thread already owns the isolate. + Dart_SetCurrentThreadOwnsIsolate(); +} + +TEST_CASE_WITH_EXPECTATION( + DartAPI_IsolateOwnership_EnterIsolateOwnedByOtherThread, + "Crash") { + Dart_Handle lib = TestCase::LoadTestScript("", nullptr); + EXPECT_VALID(lib); + + Dart_Isolate isolate = Dart_CurrentIsolate(); + + Dart_Port port = Dart_GetMainPortId(); + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port)); + + Dart_SetCurrentThreadOwnsIsolate(); + EXPECT_EQ(true, Dart_GetCurrentThreadOwnsIsolate(port)); + + Dart_ExitIsolate(); + + std::thread([isolate, port]() { + EXPECT_EQ(false, Dart_GetCurrentThreadOwnsIsolate(port)); + + // Causes an assertion failure, because the isolate is already owned by + // another thread. + Dart_EnterIsolate(isolate); + }).join(); + + Dart_EnterIsolate(isolate); +} + TEST_CASE(DartAPI_ErrorHandleBasics) { const char* kScriptChars = "@pragma('vm:entry-point', 'call')\n" diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index 7f85035d3d6..45dbeb6f35f 100644 --- a/runtime/vm/isolate.cc +++ b/runtime/vm/isolate.cc @@ -39,7 +39,6 @@ #include "vm/object_id_ring.h" #include "vm/object_store.h" #include "vm/os_thread.h" -#include "vm/port.h" #include "vm/profiler.h" #include "vm/reusable_handles.h" #include "vm/reverse_pc_lookup_cache.h" @@ -1815,6 +1814,7 @@ Isolate::Isolate(IsolateGroup* isolate_group, on_cleanup_callback_(Isolate::CleanupCallback()), random_(), mutex_(), + owner_thread_(OSThread::kInvalidThreadId), tag_table_(GrowableObjectArray::null()), sticky_error_(Error::null()), spawn_count_monitor_(), @@ -2859,6 +2859,15 @@ void Isolate::SetPrefixIsLoaded(const LibraryPrefix& prefix) { loaded_prefixes_set_storage_ = loaded_prefixes_set.Release().ptr(); } +bool Isolate::SetOwnerThread(ThreadId expected_old_owner, ThreadId new_owner) { + return owner_thread_.compare_exchange_strong(expected_old_owner, new_owner); +} + +ThreadId Isolate::GetOwnerThread(PortMap::Locker* locker) { + ASSERT(Isolate::Current() == this || locker != nullptr); + return owner_thread_.load(); +} + void IsolateGroup::EnableIncrementalBarrier( MarkingStack* old_marking_stack, MarkingStack* new_marking_stack, diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h index 209bd0e605f..ba7ac135b28 100644 --- a/runtime/vm/isolate.h +++ b/runtime/vm/isolate.h @@ -29,6 +29,7 @@ #include "vm/megamorphic_cache_table.h" #include "vm/metrics.h" #include "vm/os_thread.h" +#include "vm/port.h" #include "vm/random.h" #include "vm/service.h" #include "vm/tags.h" @@ -1478,6 +1479,12 @@ class Isolate : public IntrusiveDListEntry { return &pointers_to_verify_at_exit_; } + bool SetOwnerThread(ThreadId expected_old_owner, ThreadId new_owner); + + // Must be invoked with a valid PortMap::Locker, or while this isolate is the + // current isolate (in which case the locker may be null). + ThreadId GetOwnerThread(PortMap::Locker* locker); + private: friend class Dart; // Init, InitOnce, Shutdown. friend class IsolateKillerVisitor; // Kill(). @@ -1675,6 +1682,7 @@ class Isolate : public IntrusiveDListEntry { DeoptContext* deopt_context_ = nullptr; FfiCallbackMetadata::Metadata* ffi_callback_list_head_ = nullptr; intptr_t ffi_callback_keep_alive_counter_ = 0; + RelaxedAtomic owner_thread_ = OSThread::kInvalidThreadId; GrowableObjectArrayPtr tag_table_; diff --git a/runtime/vm/port.cc b/runtime/vm/port.cc index 547959ded28..cd5e893f76e 100644 --- a/runtime/vm/port.cc +++ b/runtime/vm/port.cc @@ -140,7 +140,7 @@ void PortMap::ClosePorts(MessageHandler* handler) { bool PortMap::PostMessage(std::unique_ptr message, bool before_events) { - MutexLocker ml(mutex_); + Locker ml; if (ports_ == nullptr) { return false; } @@ -158,17 +158,21 @@ bool PortMap::PostMessage(std::unique_ptr message, #if defined(TESTING) bool PortMap::PortExists(Dart_Port id) { - MutexLocker ml(mutex_); + Locker ml; if (ports_ == nullptr) { return false; } auto it = ports_->TryLookup(id); return it != ports_->end(); } -#endif // defined(TESTING) Isolate* PortMap::GetIsolate(Dart_Port id) { - MutexLocker ml(mutex_); + Locker ml; + return GetIsolateLocked(ml, id); +} +#endif // defined(TESTING) + +Isolate* PortMap::GetIsolateLocked(const Locker& ml, Dart_Port id) { if (ports_ == nullptr) { return nullptr; } @@ -183,7 +187,7 @@ Isolate* PortMap::GetIsolate(Dart_Port id) { } Dart_Port PortMap::GetOriginId(Dart_Port id) { - MutexLocker ml(mutex_); + Locker ml; if (ports_ == nullptr) { return ILLEGAL_PORT; } @@ -202,9 +206,19 @@ Dart_Port PortMap::GetOriginId(Dart_Port id) { return isolate->origin_id(); } +bool PortMap::IsOwnedByCurrentThread(Dart_Port id) { + Locker ml; + Isolate* isolate = GetIsolateLocked(ml, id); + if (isolate == nullptr) { + // Either the port is invalid, or the isolate has already shut down. + return false; + } + return isolate->GetOwnerThread(&ml) == OSThread::GetCurrentThreadId(); +} + #if defined(TESTING) bool PortMap::HasPorts(MessageHandler* handler) { - MutexLocker ml(mutex_); + Locker ml; if (ports_ == nullptr) { return false; } @@ -216,7 +230,7 @@ bool PortMap::HasPorts(MessageHandler* handler) { bool PortMap::IsReceiverInThisIsolateGroupOrClosed(Dart_Port receiver, IsolateGroup* group) { - MutexLocker ml(mutex_); + Locker ml; if (ports_ == nullptr) { // Port was closed. return true; @@ -266,7 +280,7 @@ void PortMap::Cleanup() { ports_->Rebalance(); // Grab the mutex and delete the port set. - MutexLocker ml(mutex_); + Locker ml; delete prng_; prng_ = nullptr; delete ports_; diff --git a/runtime/vm/port.h b/runtime/vm/port.h index 8e94e3586bd..8e05dfe3d22 100644 --- a/runtime/vm/port.h +++ b/runtime/vm/port.h @@ -43,13 +43,15 @@ class PortMap : public AllStatic { static bool PostMessage(std::unique_ptr message, bool before_events = false); - // Returns the owning Isolate for port 'id'. - static Isolate* GetIsolate(Dart_Port id); - // Returns the origin id for port 'id'. static Dart_Port GetOriginId(Dart_Port id); + // Returns whether the isolate that owns the port is owned by the current + // thread. + static bool IsOwnedByCurrentThread(Dart_Port id); + #if defined(TESTING) + static Isolate* GetIsolate(Dart_Port id); static bool PortExists(Dart_Port id); static bool HasPorts(MessageHandler* handler); #endif @@ -84,6 +86,8 @@ class PortMap : public AllStatic { // Allocate a new unique port. static Dart_Port AllocatePort(); + static Isolate* GetIsolateLocked(const Locker& ml, Dart_Port id); + // Lock protecting access to the port map. static Mutex* mutex_;