Skip to content

Commit c25ebff

Browse files
mralephmraleph
committed
[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 #28549 Bug: #28549 Change-Id: I24f27b4eef4faf4b2f80f82b269d88a6e5c3880b Reviewed-on: https://dart-review.googlesource.com/4721 Commit-Queue: Vyacheslav Egorov <[email protected]> Reviewed-by: Zach Anderson <[email protected]>
1 parent f7eba23 commit c25ebff

7 files changed

+110
-32
lines changed

‎runtime/vm/isolate.cc

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -846,11 +846,14 @@ Isolate::Isolate(const Dart_IsolateFlags& api_flags)
846846
api_state_(NULL),
847847
random_(),
848848
simulator_(NULL),
849-
mutex_(new Mutex()),
850-
symbols_mutex_(new Mutex()),
851-
type_canonicalization_mutex_(new Mutex()),
852-
constant_canonicalization_mutex_(new Mutex()),
853-
megamorphic_lookup_mutex_(new Mutex()),
849+
mutex_(new Mutex(NOT_IN_PRODUCT("Isolate::mutex_"))),
850+
symbols_mutex_(new Mutex(NOT_IN_PRODUCT("Isolate::symbols_mutex_"))),
851+
type_canonicalization_mutex_(
852+
new Mutex(NOT_IN_PRODUCT("Isolate::type_canonicalization_mutex_"))),
853+
constant_canonicalization_mutex_(new Mutex(
854+
NOT_IN_PRODUCT("Isolate::constant_canonicalization_mutex_"))),
855+
megamorphic_lookup_mutex_(
856+
new Mutex(NOT_IN_PRODUCT("Isolate::megamorphic_lookup_mutex_"))),
854857
message_handler_(NULL),
855858
spawn_state_(NULL),
856859
defer_finalization_count_(0),
@@ -862,7 +865,8 @@ Isolate::Isolate(const Dart_IsolateFlags& api_flags)
862865
next_(NULL),
863866
loading_invalidation_gen_(kInvalidGen),
864867
top_level_parsing_count_(0),
865-
field_list_mutex_(new Mutex()),
868+
field_list_mutex_(
869+
new Mutex(NOT_IN_PRODUCT("Isolate::field_list_mutex_"))),
866870
boxed_field_list_(GrowableObjectArray::null()),
867871
spawn_count_monitor_(new Monitor()),
868872
spawn_count_(0),

‎runtime/vm/os_thread.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ class OSThreadIterator : public ValueObject {
273273

274274
class Mutex {
275275
public:
276-
Mutex();
276+
explicit Mutex(NOT_IN_PRODUCT(const char* name = "anonymous mutex"));
277277
~Mutex();
278278

279279
#if defined(DEBUG)
@@ -293,6 +293,7 @@ class Mutex {
293293
void Unlock();
294294

295295
MutexData data_;
296+
NOT_IN_PRODUCT(const char* name_);
296297
#if defined(DEBUG)
297298
ThreadId owner_;
298299
#endif // defined(DEBUG)

‎runtime/vm/os_thread_android.cc

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,19 @@ namespace dart {
2828
FATAL2("pthread error: %d (%s)", result, error_message); \
2929
}
3030

31+
#if defined(PRODUCT)
32+
#define VALIDATE_PTHREAD_RESULT_NAMED(result) VALIDATE_PTHREAD_RESULT(result)
33+
#else
34+
#define VALIDATE_PTHREAD_RESULT_NAMED(result) \
35+
if (result != 0) { \
36+
const int kBufferSize = 1024; \
37+
char error_message[kBufferSize]; \
38+
NOT_IN_PRODUCT(Profiler::DumpStackTrace()); \
39+
Utils::StrError(result, error_message, kBufferSize); \
40+
FATAL3("[%s] pthread error: %d (%s)", name_, result, error_message); \
41+
}
42+
#endif
43+
3144
#if defined(DEBUG)
3245
#define ASSERT_PTHREAD_SUCCESS(result) VALIDATE_PTHREAD_RESULT(result)
3346
#else
@@ -232,22 +245,26 @@ bool OSThread::GetCurrentStackBounds(uword* lower, uword* upper) {
232245
return true;
233246
}
234247

235-
Mutex::Mutex() {
248+
Mutex::Mutex(NOT_IN_PRODUCT(const char* name))
249+
#if !defined(PRODUCT)
250+
: name_(name)
251+
#endif
252+
{
236253
pthread_mutexattr_t attr;
237254
int result = pthread_mutexattr_init(&attr);
238-
VALIDATE_PTHREAD_RESULT(result);
255+
VALIDATE_PTHREAD_RESULT_NAMED(result);
239256

240257
#if defined(DEBUG)
241258
result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK);
242-
VALIDATE_PTHREAD_RESULT(result);
259+
VALIDATE_PTHREAD_RESULT_NAMED(result);
243260
#endif // defined(DEBUG)
244261

245262
result = pthread_mutex_init(data_.mutex(), &attr);
246263
// Verify that creating a pthread_mutex succeeded.
247-
VALIDATE_PTHREAD_RESULT(result);
264+
VALIDATE_PTHREAD_RESULT_NAMED(result);
248265

249266
result = pthread_mutexattr_destroy(&attr);
250-
VALIDATE_PTHREAD_RESULT(result);
267+
VALIDATE_PTHREAD_RESULT_NAMED(result);
251268

252269
#if defined(DEBUG)
253270
// When running with assertions enabled we do track the owner.
@@ -258,7 +275,7 @@ Mutex::Mutex() {
258275
Mutex::~Mutex() {
259276
int result = pthread_mutex_destroy(data_.mutex());
260277
// Verify that the pthread_mutex was destroyed.
261-
VALIDATE_PTHREAD_RESULT(result);
278+
VALIDATE_PTHREAD_RESULT_NAMED(result);
262279

263280
#if defined(DEBUG)
264281
// When running with assertions enabled we do track the owner.

‎runtime/vm/os_thread_fuchsia.cc

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ namespace dart {
2525
FATAL1("pthread error: %d", result); \
2626
}
2727

28+
#if defined(PRODUCT)
29+
#define VALIDATE_PTHREAD_RESULT_NAMED(result) VALIDATE_PTHREAD_RESULT(result)
30+
#else
31+
#define VALIDATE_PTHREAD_RESULT_NAMED(result) \
32+
if (result != 0) { \
33+
FATAL2("[%s] pthread error: %d", name_, result); \
34+
}
35+
#endif
36+
2837
#if defined(DEBUG)
2938
#define ASSERT_PTHREAD_SUCCESS(result) VALIDATE_PTHREAD_RESULT(result)
3039
#else
@@ -200,22 +209,26 @@ bool OSThread::GetCurrentStackBounds(uword* lower, uword* upper) {
200209
return false;
201210
}
202211

203-
Mutex::Mutex() {
212+
Mutex::Mutex(NOT_IN_PRODUCT(const char* name))
213+
#if !defined(PRODUCT)
214+
: name_(name)
215+
#endif
216+
{
204217
pthread_mutexattr_t attr;
205218
int result = pthread_mutexattr_init(&attr);
206-
VALIDATE_PTHREAD_RESULT(result);
219+
VALIDATE_PTHREAD_RESULT_NAMED(result);
207220

208221
#if defined(DEBUG)
209222
result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK);
210-
VALIDATE_PTHREAD_RESULT(result);
223+
VALIDATE_PTHREAD_RESULT_NAMED(result);
211224
#endif // defined(DEBUG)
212225

213226
result = pthread_mutex_init(data_.mutex(), &attr);
214227
// Verify that creating a pthread_mutex succeeded.
215-
VALIDATE_PTHREAD_RESULT(result);
228+
VALIDATE_PTHREAD_RESULT_NAMED(result);
216229

217230
result = pthread_mutexattr_destroy(&attr);
218-
VALIDATE_PTHREAD_RESULT(result);
231+
VALIDATE_PTHREAD_RESULT_NAMED(result);
219232

220233
#if defined(DEBUG)
221234
// When running with assertions enabled we track the owner.
@@ -226,7 +239,7 @@ Mutex::Mutex() {
226239
Mutex::~Mutex() {
227240
int result = pthread_mutex_destroy(data_.mutex());
228241
// Verify that the pthread_mutex was destroyed.
229-
VALIDATE_PTHREAD_RESULT(result);
242+
VALIDATE_PTHREAD_RESULT_NAMED(result);
230243

231244
#if defined(DEBUG)
232245
// When running with assertions enabled we track the owner.

‎runtime/vm/os_thread_linux.cc

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,20 @@ namespace dart {
3030
Utils::StrError(result, error_buf, kBufferSize)); \
3131
}
3232

33+
// Variation of VALIDATE_PTHREAD_RESULT for named objects.
34+
#if defined(PRODUCT)
35+
#define VALIDATE_PTHREAD_RESULT_NAMED(result) VALIDATE_PTHREAD_RESULT(result)
36+
#else
37+
#define VALIDATE_PTHREAD_RESULT_NAMED(result) \
38+
if (result != 0) { \
39+
const int kBufferSize = 1024; \
40+
char error_buf[kBufferSize]; \
41+
NOT_IN_PRODUCT(Profiler::DumpStackTrace()); \
42+
FATAL3("[%s] pthread error: %d (%s)", name_, result, \
43+
Utils::StrError(result, error_buf, kBufferSize)); \
44+
}
45+
#endif
46+
3347
#if defined(DEBUG)
3448
#define ASSERT_PTHREAD_SUCCESS(result) VALIDATE_PTHREAD_RESULT(result)
3549
#else
@@ -233,22 +247,26 @@ bool OSThread::GetCurrentStackBounds(uword* lower, uword* upper) {
233247
return true;
234248
}
235249

236-
Mutex::Mutex() {
250+
Mutex::Mutex(NOT_IN_PRODUCT(const char* name))
251+
#if !defined(PRODUCT)
252+
: name_(name)
253+
#endif
254+
{
237255
pthread_mutexattr_t attr;
238256
int result = pthread_mutexattr_init(&attr);
239-
VALIDATE_PTHREAD_RESULT(result);
257+
VALIDATE_PTHREAD_RESULT_NAMED(result);
240258

241259
#if defined(DEBUG)
242260
result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK);
243-
VALIDATE_PTHREAD_RESULT(result);
261+
VALIDATE_PTHREAD_RESULT_NAMED(result);
244262
#endif // defined(DEBUG)
245263

246264
result = pthread_mutex_init(data_.mutex(), &attr);
247265
// Verify that creating a pthread_mutex succeeded.
248-
VALIDATE_PTHREAD_RESULT(result);
266+
VALIDATE_PTHREAD_RESULT_NAMED(result);
249267

250268
result = pthread_mutexattr_destroy(&attr);
251-
VALIDATE_PTHREAD_RESULT(result);
269+
VALIDATE_PTHREAD_RESULT_NAMED(result);
252270

253271
#if defined(DEBUG)
254272
// When running with assertions enabled we track the owner.
@@ -259,7 +277,7 @@ Mutex::Mutex() {
259277
Mutex::~Mutex() {
260278
int result = pthread_mutex_destroy(data_.mutex());
261279
// Verify that the pthread_mutex was destroyed.
262-
VALIDATE_PTHREAD_RESULT(result);
280+
VALIDATE_PTHREAD_RESULT_NAMED(result);
263281

264282
#if defined(DEBUG)
265283
// When running with assertions enabled we track the owner.

‎runtime/vm/os_thread_macos.cc

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,19 @@ namespace dart {
3434
FATAL2("pthread error: %d (%s)", result, error_message); \
3535
}
3636

37+
#if defined(PRODUCT)
38+
#define VALIDATE_PTHREAD_RESULT_NAMED(result) VALIDATE_PTHREAD_RESULT(result)
39+
#else
40+
#define VALIDATE_PTHREAD_RESULT_NAMED(result) \
41+
if (result != 0) { \
42+
const int kBufferSize = 1024; \
43+
char error_message[kBufferSize]; \
44+
NOT_IN_PRODUCT(Profiler::DumpStackTrace()); \
45+
Utils::StrError(result, error_message, kBufferSize); \
46+
FATAL3("[%s] pthread error: %d (%s)", name_, result, error_message); \
47+
}
48+
#endif
49+
3750
#if defined(DEBUG)
3851
#define ASSERT_PTHREAD_SUCCESS(result) VALIDATE_PTHREAD_RESULT(result)
3952
#else
@@ -197,22 +210,26 @@ bool OSThread::GetCurrentStackBounds(uword* lower, uword* upper) {
197210
return true;
198211
}
199212

200-
Mutex::Mutex() {
213+
Mutex::Mutex(NOT_IN_PRODUCT(const char* name))
214+
#if !defined(PRODUCT)
215+
: name_(name)
216+
#endif
217+
{
201218
pthread_mutexattr_t attr;
202219
int result = pthread_mutexattr_init(&attr);
203-
VALIDATE_PTHREAD_RESULT(result);
220+
VALIDATE_PTHREAD_RESULT_NAMED(result);
204221

205222
#if defined(DEBUG)
206223
result = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK);
207-
VALIDATE_PTHREAD_RESULT(result);
224+
VALIDATE_PTHREAD_RESULT_NAMED(result);
208225
#endif // defined(DEBUG)
209226

210227
result = pthread_mutex_init(data_.mutex(), &attr);
211228
// Verify that creating a pthread_mutex succeeded.
212-
VALIDATE_PTHREAD_RESULT(result);
229+
VALIDATE_PTHREAD_RESULT_NAMED(result);
213230

214231
result = pthread_mutexattr_destroy(&attr);
215-
VALIDATE_PTHREAD_RESULT(result);
232+
VALIDATE_PTHREAD_RESULT_NAMED(result);
216233

217234
#if defined(DEBUG)
218235
// When running with assertions enabled we do track the owner.
@@ -223,7 +240,7 @@ Mutex::Mutex() {
223240
Mutex::~Mutex() {
224241
int result = pthread_mutex_destroy(data_.mutex());
225242
// Verify that the pthread_mutex was destroyed.
226-
VALIDATE_PTHREAD_RESULT(result);
243+
VALIDATE_PTHREAD_RESULT_NAMED(result);
227244

228245
#if defined(DEBUG)
229246
// When running with assertions enabled we do track the owner.

‎runtime/vm/os_thread_win.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,19 @@ void OSThread::SetThreadLocal(ThreadLocalKey key, uword value) {
181181
}
182182
}
183183

184-
Mutex::Mutex() {
184+
Mutex::Mutex(NOT_IN_PRODUCT(const char* name))
185+
#if !defined(PRODUCT)
186+
: name_(name)
187+
#endif
188+
{
185189
// Allocate unnamed semaphore with initial count 1 and max count 1.
186190
data_.semaphore_ = CreateSemaphore(NULL, 1, 1, NULL);
187191
if (data_.semaphore_ == NULL) {
192+
#if defined(PRODUCT)
188193
FATAL1("Mutex allocation failed %d", GetLastError());
194+
#else
195+
FATAL2("[%s] Mutex allocation failed %d", name_, GetLastError());
196+
#endif
189197
}
190198
#if defined(DEBUG)
191199
// When running with assertions enabled we do track the owner.

0 commit comments

Comments
 (0)