Fix a few issues breaking cctest/test-lockers/Regress1433:

o The thread local state in an isolate has to be initialized before
  it's used.
o v8::Locker was incorrectly tracking whether it's the topmost one.
o Waking the profiler thread on shutdown should not leave the
  semaphore counter in an inconsitent state.

R=fschneider@chromium.org
BUG=v8:1522
TEST=cctest/test-lockers/Regress1433

Review URL: http://codereview.chromium.org/7309013

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8537 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/execution.cc b/src/execution.cc
index 990eca2..6ab73e7 100644 (file)
--- a/src/execution.cc
+++ b/src/execution.cc
@@ -452,8 +452,9 @@ char* StackGuard::RestoreStackGuard(char* from) {
 
 
 void StackGuard::FreeThreadResources() {
-  Isolate::CurrentPerIsolateThreadData()->set_stack_limit(
-      thread_local_.real_climit_);
+  Isolate::PerIsolateThreadData* per_thread =
+      isolate_->FindOrAllocatePerThreadDataForThisThread();
+  per_thread->set_stack_limit(thread_local_.real_climit_);
 }
 
 
@@ -502,7 +503,7 @@ void StackGuard::InitThread(const ExecutionAccess& lock) {
   uintptr_t stored_limit = per_thread->stack_limit();
   // You should hold the ExecutionAccess lock when you call this.
   if (stored_limit != 0) {
-    StackGuard::SetStackLimit(stored_limit);
+    SetStackLimit(stored_limit);
   }
 }
 

diff --git a/src/isolate.cc b/src/isolate.cc
index 01f457b..7423274 100644 (file)
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -1731,11 +1731,11 @@ bool Isolate::Init(Deserializer* des) {
     return false;
   }
 
+  InitializeThreadLocal();
+
   bootstrapper_->Initialize(create_heap_objects);
   builtins_.Setup(create_heap_objects);
 
-  InitializeThreadLocal();
-
   // Only preallocate on the first initialization.
   if (FLAG_preallocate_message_memory && preallocated_message_space_ == NULL) {
     // Start the thread which will set aside some memory.

diff --git a/src/isolate.h b/src/isolate.h
index be8141a..a4af136 100644 (file)
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -961,6 +961,8 @@ class Isolate {
 
   void SetCurrentVMState(StateTag state) {
     if (RuntimeProfiler::IsEnabled()) {
+      // Make sure thread local top is initialized.
+      ASSERT(thread_local_top_.isolate_ == this);
       StateTag current_state = thread_local_top_.current_vm_state_;
       if (current_state != JS && state == JS) {
         // Non-JS -> JS transition.

diff --git a/src/platform-cygwin.cc b/src/platform-cygwin.cc
index b99d735..0242f7b 100644 (file)
--- a/src/platform-cygwin.cc
+++ b/src/platform-cygwin.cc
@@ -646,8 +646,7 @@ class SamplerThread : public Thread {
     ScopedLock lock(mutex_);
     SamplerRegistry::RemoveActiveSampler(sampler);
     if (SamplerRegistry::GetState() == SamplerRegistry::HAS_NO_SAMPLERS) {
-      RuntimeProfiler::WakeUpRuntimeProfilerThreadBeforeShutdown();
-      instance_->Join();
+      RuntimeProfiler::StopRuntimeProfilerThreadBeforeShutdown(instance_);
       delete instance_;
       instance_ = NULL;
     }

diff --git a/src/platform-freebsd.cc b/src/platform-freebsd.cc
index 1eefaa3..755475a 100644 (file)
--- a/src/platform-freebsd.cc
+++ b/src/platform-freebsd.cc
@@ -684,8 +684,7 @@ class SignalSender : public Thread {
     ScopedLock lock(mutex_);
     SamplerRegistry::RemoveActiveSampler(sampler);
     if (SamplerRegistry::GetState() == SamplerRegistry::HAS_NO_SAMPLERS) {
-      RuntimeProfiler::WakeUpRuntimeProfilerThreadBeforeShutdown();
-      instance_->Join();
+      RuntimeProfiler::StopRuntimeProfilerThreadBeforeShutdown(instance_);
       delete instance_;
       instance_ = NULL;
 

diff --git a/src/platform-linux.cc b/src/platform-linux.cc
index 228d6f4..d2866ca 100644 (file)
--- a/src/platform-linux.cc
+++ b/src/platform-linux.cc
@@ -1008,8 +1008,7 @@ class SignalSender : public Thread {
     ScopedLock lock(mutex_);
     SamplerRegistry::RemoveActiveSampler(sampler);
     if (SamplerRegistry::GetState() == SamplerRegistry::HAS_NO_SAMPLERS) {
-      RuntimeProfiler::WakeUpRuntimeProfilerThreadBeforeShutdown();
-      instance_->Join();
+      RuntimeProfiler::StopRuntimeProfilerThreadBeforeShutdown(instance_);
       delete instance_;
       instance_ = NULL;
       RestoreSignalHandler();

diff --git a/src/platform-macos.cc b/src/platform-macos.cc
index cdbbb12..104729a 100644 (file)
--- a/src/platform-macos.cc
+++ b/src/platform-macos.cc
@@ -685,8 +685,7 @@ class SamplerThread : public Thread {
     ScopedLock lock(mutex_);
     SamplerRegistry::RemoveActiveSampler(sampler);
     if (SamplerRegistry::GetState() == SamplerRegistry::HAS_NO_SAMPLERS) {
-      RuntimeProfiler::WakeUpRuntimeProfilerThreadBeforeShutdown();
-      instance_->Join();
+      RuntimeProfiler::StopRuntimeProfilerThreadBeforeShutdown(instance_);
       delete instance_;
       instance_ = NULL;
     }

diff --git a/src/platform-openbsd.cc b/src/platform-openbsd.cc
index 6034800..ceabb51 100644 (file)
--- a/src/platform-openbsd.cc
+++ b/src/platform-openbsd.cc
@@ -685,8 +685,7 @@ class SignalSender : public Thread {
     ScopedLock lock(mutex_);
     SamplerRegistry::RemoveActiveSampler(sampler);
     if (SamplerRegistry::GetState() == SamplerRegistry::HAS_NO_SAMPLERS) {
-      RuntimeProfiler::WakeUpRuntimeProfilerThreadBeforeShutdown();
-      instance_->Join();
+      RuntimeProfiler::StopRuntimeProfilerThreadBeforeShutdown(instance_);
       delete instance_;
       instance_ = NULL;
 

diff --git a/src/platform-solaris.cc b/src/platform-solaris.cc
index bbd982c..ca15b07 100644 (file)
--- a/src/platform-solaris.cc
+++ b/src/platform-solaris.cc
@@ -683,8 +683,7 @@ class SignalSender : public Thread {
     ScopedLock lock(mutex_);
     SamplerRegistry::RemoveActiveSampler(sampler);
     if (SamplerRegistry::GetState() == SamplerRegistry::HAS_NO_SAMPLERS) {
-      RuntimeProfiler::WakeUpRuntimeProfilerThreadBeforeShutdown();
-      instance_->Join();
+      RuntimeProfiler::StopRuntimeProfilerThreadBeforeShutdown(instance_);
       delete instance_;
       instance_ = NULL;
       RestoreSignalHandler();

diff --git a/src/platform-win32.cc b/src/platform-win32.cc
index b7eed47..c226e2f 100644 (file)
--- a/src/platform-win32.cc
+++ b/src/platform-win32.cc
@@ -1888,8 +1888,7 @@ class SamplerThread : public Thread {
     ScopedLock lock(mutex_);
     SamplerRegistry::RemoveActiveSampler(sampler);
     if (SamplerRegistry::GetState() == SamplerRegistry::HAS_NO_SAMPLERS) {
-      RuntimeProfiler::WakeUpRuntimeProfilerThreadBeforeShutdown();
-      instance_->Join();
+      RuntimeProfiler::StopRuntimeProfilerThreadBeforeShutdown(instance_);
       delete instance_;
       instance_ = NULL;
     }

diff --git a/src/runtime-profiler.cc b/src/runtime-profiler.cc
index c0eaf98..7a0dd91 100644 (file)
--- a/src/runtime-profiler.cc
+++ b/src/runtime-profiler.cc
@@ -323,9 +323,26 @@ bool RuntimeProfiler::WaitForSomeIsolateToEnterJS() {
 }
 
 
-void RuntimeProfiler::WakeUpRuntimeProfilerThreadBeforeShutdown() {
+void RuntimeProfiler::StopRuntimeProfilerThreadBeforeShutdown(Thread* thread) {
 #ifdef ENABLE_LOGGING_AND_PROFILING
-  semaphore_->Signal();
+  // Do a fake increment. If the profiler is waiting on the semaphore,
+  // the returned state is 0, which can be left as an initial state in
+  // case profiling is restarted later. If the profiler is not
+  // waiting, the increment will prevent it from waiting, but has to
+  // be undone after the profiler is stopped.
+  Atomic32 new_state = NoBarrier_AtomicIncrement(&state_, 1);
+  ASSERT(new_state >= 0);
+  if (new_state == 0) {
+    // The profiler thread is waiting. Wake it up. It must check for
+    // stop conditions before attempting to wait again.
+    semaphore_->Signal();
+  }
+  thread->Join();
+  // The profiler thread is now stopped. Undo the increment in case it
+  // was not waiting.
+  if (new_state != 0) {
+    NoBarrier_AtomicIncrement(&state_, -1);
+  }
 #endif
 }
 

diff --git a/src/runtime-profiler.h b/src/runtime-profiler.h
index cb05cf6..3f3ab07 100644 (file)
--- a/src/runtime-profiler.h
+++ b/src/runtime-profiler.h
@@ -84,10 +84,9 @@ class RuntimeProfiler {
   static bool IsSomeIsolateInJS();
   static bool WaitForSomeIsolateToEnterJS();
 
-  // When shutting down we join the profiler thread. Doing so while
-  // it's waiting on a semaphore will cause a deadlock, so we have to
-  // wake it up first.
-  static void WakeUpRuntimeProfilerThreadBeforeShutdown();
+  // Stops the runtime profiler thread when profiling support is being
+  // turned off.
+  static void StopRuntimeProfilerThreadBeforeShutdown(Thread* thread);
 
   void UpdateSamplesAfterScavenge();
   void RemoveDeadSamples();

diff --git a/src/v8threads.cc b/src/v8threads.cc
index 978e2dd..c9a8bb6 100644 (file)
--- a/src/v8threads.cc
+++ b/src/v8threads.cc
@@ -46,7 +46,7 @@ bool Locker::active_ = false;
 // current thread will be guaranteed to have the lock for a given isolate.
 Locker::Locker(v8::Isolate* isolate)
   : has_lock_(false),
-    top_level_(false),
+    top_level_(true),
     isolate_(reinterpret_cast<i::Isolate*>(isolate)) {
   if (isolate_ == NULL) {
     isolate_ = i::Isolate::GetDefaultIsolateForLocking();