From bbaed5b82e3531506481fd89e531da9e3c74a9c7 Mon Sep 17 00:00:00 2001 From: "vitalyr@chromium.org" Date: Tue, 5 Jul 2011 15:49:39 +0000 Subject: [PATCH] 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 --- src/execution.cc | 7 ++++--- src/isolate.cc | 4 ++-- src/isolate.h | 2 ++ src/platform-cygwin.cc | 3 +-- src/platform-freebsd.cc | 3 +-- src/platform-linux.cc | 3 +-- src/platform-macos.cc | 3 +-- src/platform-openbsd.cc | 3 +-- src/platform-solaris.cc | 3 +-- src/platform-win32.cc | 3 +-- src/runtime-profiler.cc | 21 +++++++++++++++++++-- src/runtime-profiler.h | 7 +++---- src/v8threads.cc | 2 +- 13 files changed, 38 insertions(+), 26 deletions(-) diff --git a/src/execution.cc b/src/execution.cc index 990eca2e5b..6ab73e760e 100644 --- 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 01f457b93a..7423274a1e 100644 --- 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 be8141a37b..a4af1362a3 100644 --- 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 b99d73584d..0242f7b2bd 100644 --- 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 1eefaa3969..755475a8a0 100644 --- 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 228d6f4d13..d2866cae45 100644 --- 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 cdbbb12d4a..104729af5d 100644 --- 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 6034800f75..ceabb51f10 100644 --- 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 bbd982c319..ca15b07f11 100644 --- 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 b7eed47cba..c226e2f3f8 100644 --- 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 c0eaf98779..7a0dd91f70 100644 --- 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 cb05cf6b16..3f3ab0773a 100644 --- 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 978e2ddf3f..c9a8bb6fa2 100644 --- 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(isolate)) { if (isolate_ == NULL) { isolate_ = i::Isolate::GetDefaultIsolateForLocking();