From c25ebfff962928b4ab599d6f7e0ffc2087639b4a Mon Sep 17 00:00:00 2001 From: Vyacheslav Egorov Date: Mon, 11 Sep 2017 19:17:46 +0000 Subject: [PATCH] [VM, Platform] Allow assigning names to mutexes. Use this name when reporting errors. We need this sort of debug information to better understand Isolate shutdown races like one in https://github.com/dart-lang/sdk/issues/28549 Bug: https://github.com/dart-lang/sdk/issues/28549 Change-Id: I24f27b4eef4faf4b2f80f82b269d88a6e5c3880b Reviewed-on: https://dart-review.googlesource.com/4721 Commit-Queue: Vyacheslav Egorov Reviewed-by: Zach Anderson --- runtime/vm/isolate.cc | 16 ++++++++++------ runtime/vm/os_thread.h | 3 ++- runtime/vm/os_thread_android.cc | 29 +++++++++++++++++++++++------ runtime/vm/os_thread_fuchsia.cc | 25 +++++++++++++++++++------ runtime/vm/os_thread_linux.cc | 30 ++++++++++++++++++++++++------ runtime/vm/os_thread_macos.cc | 29 +++++++++++++++++++++++------ runtime/vm/os_thread_win.cc | 10 +++++++++- 7 files changed, 110 insertions(+), 32 deletions(-) diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index d7be149e837..76448e69ef8 100644 --- a/runtime/vm/isolate.cc +++ b/runtime/vm/isolate.cc @@ -846,11 +846,14 @@ Isolate::Isolate(const Dart_IsolateFlags& api_flags) api_state_(NULL), random_(), simulator_(NULL), - mutex_(new Mutex()), - symbols_mutex_(new Mutex()), - type_canonicalization_mutex_(new Mutex()), - constant_canonicalization_mutex_(new Mutex()), - megamorphic_lookup_mutex_(new Mutex()), + mutex_(new Mutex(NOT_IN_PRODUCT("Isolate::mutex_"))), + symbols_mutex_(new Mutex(NOT_IN_PRODUCT("Isolate::symbols_mutex_"))), + type_canonicalization_mutex_( + new Mutex(NOT_IN_PRODUCT("Isolate::type_canonicalization_mutex_"))), + constant_canonicalization_mutex_(new Mutex( + NOT_IN_PRODUCT("Isolate::constant_canonicalization_mutex_"))), + megamorphic_lookup_mutex_( + new Mutex(NOT_IN_PRODUCT("Isolate::megamorphic_lookup_mutex_"))), message_handler_(NULL), spawn_state_(NULL), defer_finalization_count_(0), @@ -862,7 +865,8 @@ Isolate::Isolate(const Dart_IsolateFlags& api_flags) next_(NULL), loading_invalidation_gen_(kInvalidGen), top_level_parsing_count_(0), - field_list_mutex_(new Mutex()), + field_list_mutex_( + new Mutex(NOT_IN_PRODUCT("Isolate::field_list_mutex_"))), boxed_field_list_(GrowableObjectArray::null()), spawn_count_monitor_(new Monitor()), spawn_count_(0), diff --git a/runtime/vm/os_thread.h b/runtime/vm/os_thread.h index b7f70d0a035..db75c7a9f59 100644 --- a/runtime/vm/os_thread.h +++ b/runtime/vm/os_thread.h @@ -273,7 +273,7 @@ class OSThreadIterator : public ValueObject { class Mutex { public: - Mutex(); + explicit Mutex(NOT_IN_PRODUCT(const char* name = "anonymous mutex")); ~Mutex(); #if defined(DEBUG) @@ -293,6 +293,7 @@ class Mutex { void Unlock(); MutexData data_; + NOT_IN_PRODUCT(const char* name_); #if defined(DEBUG) ThreadId owner_; #endif // defined(DEBUG) diff --git a/runtime/vm/os_thread_android.cc b/runtime/vm/os_thread_android.cc index dc09b5eea93..55073615705 100644 --- a/runtime/vm/os_thread_android.cc +++ b/runtime/vm/os_thread_android.cc @@ -28,6 +28,19 @@ namespace dart { FATAL2("pthread error: %d (%s)", result, error_message); \ } +#if defined(PRODUCT) +#define VALIDATE_PTHREAD_RESULT_NAMED(result) VALIDATE_PTHREAD_RESULT(result) +#else +#define VALIDATE_PTHREAD_RESULT_NAMED(result) \ + if (result != 0) { \ + const int kBufferSize = 1024; \ + char error_message[kBufferSize]; \ + NOT_IN_PRODUCT(Profiler::DumpStackTrace()); \ + Utils::StrError(result, error_message, kBufferSize); \ + FATAL3("[%s] pthread error: %d (%s)", name_, result, error_message); \ + } +#endif + #if defined(DEBUG) #define ASSERT_PTHREAD_SUCCESS(result) VALIDATE_PTHREAD_RESULT(result) #else @@ -232,22 +245,26 @@ bool OSThread::GetCurrentStackBounds(uword* lower, uword* upper) { return true; } -Mutex::Mutex() { +Mutex::Mutex(NOT_IN_PRODUCT(const char* name)) +#if !defined(PRODUCT) + : name_(name) +#endif +{ pthread_mutexattr_t attr; int result = pthread_mutexattr_init(&attr); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #endif // defined(DEBUG) result = pthread_mutex_init(data_.mutex(), &attr); // Verify that creating a pthread_mutex succeeded. - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); result = pthread_mutexattr_destroy(&attr); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) // When running with assertions enabled we do track the owner. @@ -258,7 +275,7 @@ Mutex::Mutex() { Mutex::~Mutex() { int result = pthread_mutex_destroy(data_.mutex()); // Verify that the pthread_mutex was destroyed. - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) // When running with assertions enabled we do track the owner. diff --git a/runtime/vm/os_thread_fuchsia.cc b/runtime/vm/os_thread_fuchsia.cc index 2deaa933b0b..313c4dbdfc1 100644 --- a/runtime/vm/os_thread_fuchsia.cc +++ b/runtime/vm/os_thread_fuchsia.cc @@ -25,6 +25,15 @@ namespace dart { FATAL1("pthread error: %d", result); \ } +#if defined(PRODUCT) +#define VALIDATE_PTHREAD_RESULT_NAMED(result) VALIDATE_PTHREAD_RESULT(result) +#else +#define VALIDATE_PTHREAD_RESULT_NAMED(result) \ + if (result != 0) { \ + FATAL2("[%s] pthread error: %d", name_, result); \ + } +#endif + #if defined(DEBUG) #define ASSERT_PTHREAD_SUCCESS(result) VALIDATE_PTHREAD_RESULT(result) #else @@ -200,22 +209,26 @@ bool OSThread::GetCurrentStackBounds(uword* lower, uword* upper) { return false; } -Mutex::Mutex() { +Mutex::Mutex(NOT_IN_PRODUCT(const char* name)) +#if !defined(PRODUCT) + : name_(name) +#endif +{ pthread_mutexattr_t attr; int result = pthread_mutexattr_init(&attr); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #endif // defined(DEBUG) result = pthread_mutex_init(data_.mutex(), &attr); // Verify that creating a pthread_mutex succeeded. - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); result = pthread_mutexattr_destroy(&attr); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) // When running with assertions enabled we track the owner. @@ -226,7 +239,7 @@ Mutex::Mutex() { Mutex::~Mutex() { int result = pthread_mutex_destroy(data_.mutex()); // Verify that the pthread_mutex was destroyed. - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) // When running with assertions enabled we track the owner. diff --git a/runtime/vm/os_thread_linux.cc b/runtime/vm/os_thread_linux.cc index 3b5239da5f6..68f16c91937 100644 --- a/runtime/vm/os_thread_linux.cc +++ b/runtime/vm/os_thread_linux.cc @@ -30,6 +30,20 @@ namespace dart { Utils::StrError(result, error_buf, kBufferSize)); \ } +// Variation of VALIDATE_PTHREAD_RESULT for named objects. +#if defined(PRODUCT) +#define VALIDATE_PTHREAD_RESULT_NAMED(result) VALIDATE_PTHREAD_RESULT(result) +#else +#define VALIDATE_PTHREAD_RESULT_NAMED(result) \ + if (result != 0) { \ + const int kBufferSize = 1024; \ + char error_buf[kBufferSize]; \ + NOT_IN_PRODUCT(Profiler::DumpStackTrace()); \ + FATAL3("[%s] pthread error: %d (%s)", name_, result, \ + Utils::StrError(result, error_buf, kBufferSize)); \ + } +#endif + #if defined(DEBUG) #define ASSERT_PTHREAD_SUCCESS(result) VALIDATE_PTHREAD_RESULT(result) #else @@ -233,22 +247,26 @@ bool OSThread::GetCurrentStackBounds(uword* lower, uword* upper) { return true; } -Mutex::Mutex() { +Mutex::Mutex(NOT_IN_PRODUCT(const char* name)) +#if !defined(PRODUCT) + : name_(name) +#endif +{ pthread_mutexattr_t attr; int result = pthread_mutexattr_init(&attr); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #endif // defined(DEBUG) result = pthread_mutex_init(data_.mutex(), &attr); // Verify that creating a pthread_mutex succeeded. - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); result = pthread_mutexattr_destroy(&attr); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) // When running with assertions enabled we track the owner. @@ -259,7 +277,7 @@ Mutex::Mutex() { Mutex::~Mutex() { int result = pthread_mutex_destroy(data_.mutex()); // Verify that the pthread_mutex was destroyed. - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) // When running with assertions enabled we track the owner. diff --git a/runtime/vm/os_thread_macos.cc b/runtime/vm/os_thread_macos.cc index ec7b3942fa9..7bc45d5cc14 100644 --- a/runtime/vm/os_thread_macos.cc +++ b/runtime/vm/os_thread_macos.cc @@ -34,6 +34,19 @@ namespace dart { FATAL2("pthread error: %d (%s)", result, error_message); \ } +#if defined(PRODUCT) +#define VALIDATE_PTHREAD_RESULT_NAMED(result) VALIDATE_PTHREAD_RESULT(result) +#else +#define VALIDATE_PTHREAD_RESULT_NAMED(result) \ + if (result != 0) { \ + const int kBufferSize = 1024; \ + char error_message[kBufferSize]; \ + NOT_IN_PRODUCT(Profiler::DumpStackTrace()); \ + Utils::StrError(result, error_message, kBufferSize); \ + FATAL3("[%s] pthread error: %d (%s)", name_, result, error_message); \ + } +#endif + #if defined(DEBUG) #define ASSERT_PTHREAD_SUCCESS(result) VALIDATE_PTHREAD_RESULT(result) #else @@ -197,22 +210,26 @@ bool OSThread::GetCurrentStackBounds(uword* lower, uword* upper) { return true; } -Mutex::Mutex() { +Mutex::Mutex(NOT_IN_PRODUCT(const char* name)) +#if !defined(PRODUCT) + : name_(name) +#endif +{ pthread_mutexattr_t attr; int result = pthread_mutexattr_init(&attr); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #endif // defined(DEBUG) result = pthread_mutex_init(data_.mutex(), &attr); // Verify that creating a pthread_mutex succeeded. - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); result = pthread_mutexattr_destroy(&attr); - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) // When running with assertions enabled we do track the owner. @@ -223,7 +240,7 @@ Mutex::Mutex() { Mutex::~Mutex() { int result = pthread_mutex_destroy(data_.mutex()); // Verify that the pthread_mutex was destroyed. - VALIDATE_PTHREAD_RESULT(result); + VALIDATE_PTHREAD_RESULT_NAMED(result); #if defined(DEBUG) // When running with assertions enabled we do track the owner. diff --git a/runtime/vm/os_thread_win.cc b/runtime/vm/os_thread_win.cc index dcd44eabe77..321fbdae4d3 100644 --- a/runtime/vm/os_thread_win.cc +++ b/runtime/vm/os_thread_win.cc @@ -181,11 +181,19 @@ void OSThread::SetThreadLocal(ThreadLocalKey key, uword value) { } } -Mutex::Mutex() { +Mutex::Mutex(NOT_IN_PRODUCT(const char* name)) +#if !defined(PRODUCT) + : name_(name) +#endif +{ // Allocate unnamed semaphore with initial count 1 and max count 1. data_.semaphore_ = CreateSemaphore(NULL, 1, 1, NULL); if (data_.semaphore_ == NULL) { +#if defined(PRODUCT) FATAL1("Mutex allocation failed %d", GetLastError()); +#else + FATAL2("[%s] Mutex allocation failed %d", name_, GetLastError()); +#endif } #if defined(DEBUG) // When running with assertions enabled we do track the owner.