Fix race condition with HasMutatorThread that was being used without a lock.
R=fschneider@google.com
Review URL: https://codereview.chromium.org/2191723002 .
diff --git a/runtime/vm/compiler.cc b/runtime/vm/compiler.cc
index 1bc179c..ca099b5 100644
--- a/runtime/vm/compiler.cc
+++ b/runtime/vm/compiler.cc
@@ -1874,10 +1874,7 @@
ASSERT(error.IsNull());
#ifndef PRODUCT
Isolate* isolate = thread->isolate();
- // We cannot aggregate stats if isolate is shutting down.
- if (isolate->HasMutatorThread()) {
- isolate->aggregate_compiler_stats()->Add(*thread->compiler_stats());
- }
+ isolate->aggregate_compiler_stats()->Add(*thread->compiler_stats());
thread->compiler_stats()->Clear();
#endif // PRODUCT
diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc
index 34a8d33..66894b5 100644
--- a/runtime/vm/dart_api_impl.cc
+++ b/runtime/vm/dart_api_impl.cc
@@ -1344,11 +1344,10 @@
CHECK_NO_ISOLATE(Isolate::Current());
// TODO(16615): Validate isolate parameter.
Isolate* iso = reinterpret_cast<Isolate*>(isolate);
- if (iso->HasMutatorThread()) {
- FATAL("Multiple mutators within one isolate is not supported.");
- }
if (!Thread::EnterIsolate(iso)) {
- FATAL("Unable to Enter Isolate as Dart VM is shutting down");
+ FATAL("Unable to Enter Isolate : "
+ "Multiple mutators entering an isolate / "
+ "Dart VM is shutting down");
}
// A Thread structure has been associated to the thread, we do the
// safepoint transition explicity here instead of using the
diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index 697aef8..15091b5 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -989,6 +989,12 @@
}
+Thread* Isolate::mutator_thread() const {
+ ASSERT(thread_registry() != NULL);
+ return thread_registry()->mutator_thread();
+}
+
+
void Isolate::SetupInstructionsSnapshotPage(
const uint8_t* instructions_snapshot_buffer) {
InstructionsSnapshot snapshot(instructions_snapshot_buffer);
@@ -2585,6 +2591,11 @@
// no_safepoint_scope_depth increments/decrements.
MonitorLocker ml(threads_lock(), false);
+ // Check to make sure we don't already have a mutator thread.
+ if (is_mutator && mutator_thread_ != NULL) {
+ return NULL;
+ }
+
// If a safepoint operation is in progress wait for it
// to finish before scheduling this thread in.
while (!bypass_safepoint && safepoint_handler()->SafepointInProgress()) {
diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h
index b49d59b..9761071 100644
--- a/runtime/vm/isolate.h
+++ b/runtime/vm/isolate.h
@@ -177,15 +177,7 @@
message_notify_callback_ = value;
}
- // Limited public access to BaseIsolate::mutator_thread_ for code that
- // must treat the mutator as the default or a special case. Prefer code
- // that works uniformly across all threads.
- bool HasMutatorThread() {
- return mutator_thread_ != NULL;
- }
- Thread* mutator_thread() const {
- return mutator_thread_;
- }
+ Thread* mutator_thread() const;
const char* name() const { return name_; }
const char* debugger_name() const { return debugger_name_; }
@@ -466,8 +458,7 @@
// Mutator thread is used to aggregate compiler stats.
CompilerStats* aggregate_compiler_stats() {
- ASSERT(HasMutatorThread());
- return mutator_thread_->compiler_stats();
+ return mutator_thread()->compiler_stats();
}
VMTagCounters* vm_tag_counters() {
@@ -693,8 +684,8 @@
// DEPRECATED: Use Thread's methods instead. During migration, these default
// to using the mutator thread (which must also be the current thread).
Zone* current_zone() const {
- ASSERT(Thread::Current() == mutator_thread_);
- return mutator_thread_->zone();
+ ASSERT(Thread::Current() == mutator_thread());
+ return mutator_thread()->zone();
}
// Accessed from generated code:
diff --git a/runtime/vm/thread_registry.h b/runtime/vm/thread_registry.h
index adf2a34..dd2c6df 100644
--- a/runtime/vm/thread_registry.h
+++ b/runtime/vm/thread_registry.h
@@ -26,6 +26,7 @@
void VisitObjectPointers(ObjectPointerVisitor* visitor, bool validate_frames);
void PrepareForGC();
+ Thread* mutator_thread() const { return mutator_thread_; }
private:
Thread* active_list() const { return active_list_; }
diff --git a/runtime/vm/virtual_memory_android.cc b/runtime/vm/virtual_memory_android.cc
index f84e2ef..16aa834 100644
--- a/runtime/vm/virtual_memory_android.cc
+++ b/runtime/vm/virtual_memory_android.cc
@@ -83,7 +83,6 @@
bool VirtualMemory::Protect(void* address, intptr_t size, Protection mode) {
ASSERT(Thread::Current()->IsMutatorThread() ||
- !Isolate::Current()->HasMutatorThread() ||
Isolate::Current()->mutator_thread()->IsAtSafepoint());
uword start_address = reinterpret_cast<uword>(address);
uword end_address = start_address + size;
diff --git a/runtime/vm/virtual_memory_linux.cc b/runtime/vm/virtual_memory_linux.cc
index 8973345..0228f68 100644
--- a/runtime/vm/virtual_memory_linux.cc
+++ b/runtime/vm/virtual_memory_linux.cc
@@ -82,7 +82,6 @@
bool VirtualMemory::Protect(void* address, intptr_t size, Protection mode) {
ASSERT(Thread::Current()->IsMutatorThread() ||
- !Isolate::Current()->HasMutatorThread() ||
Isolate::Current()->mutator_thread()->IsAtSafepoint());
uword start_address = reinterpret_cast<uword>(address);
uword end_address = start_address + size;
diff --git a/runtime/vm/virtual_memory_macos.cc b/runtime/vm/virtual_memory_macos.cc
index 1d23ead..484ad78 100644
--- a/runtime/vm/virtual_memory_macos.cc
+++ b/runtime/vm/virtual_memory_macos.cc
@@ -83,7 +83,6 @@
bool VirtualMemory::Protect(void* address, intptr_t size, Protection mode) {
ASSERT(Thread::Current()->IsMutatorThread() ||
- !Isolate::Current()->HasMutatorThread() ||
Isolate::Current()->mutator_thread()->IsAtSafepoint());
uword start_address = reinterpret_cast<uword>(address);
uword end_address = start_address + size;
diff --git a/runtime/vm/virtual_memory_win.cc b/runtime/vm/virtual_memory_win.cc
index 8ea8fa5..2584613 100644
--- a/runtime/vm/virtual_memory_win.cc
+++ b/runtime/vm/virtual_memory_win.cc
@@ -66,7 +66,6 @@
bool VirtualMemory::Protect(void* address, intptr_t size, Protection mode) {
ASSERT(Thread::Current()->IsMutatorThread() ||
- !Isolate::Current()->HasMutatorThread() ||
Isolate::Current()->mutator_thread()->IsAtSafepoint());
uword start_address = reinterpret_cast<uword>(address);
uword end_address = start_address + size;
