Revision: 2825 Author: [email protected] Date: Fri Sep 4 04:16:51 2009 Log: Linux profiler: check whether signal handler is called in the VM thread.
I have several Chromium's core files having SIGPROF signal handler called in the context of an arbitrary thread, causing a crash. This change introduces checking of current thread in the signal handler. Review URL: http://codereview.chromium.org/171115 http://code.google.com/p/v8/source/detail?r=2825 Modified: /branches/bleeding_edge/src/platform-linux.cc /branches/bleeding_edge/src/v8threads.cc /branches/bleeding_edge/src/v8threads.h /branches/bleeding_edge/test/cctest/test-log.cc ======================================= --- /branches/bleeding_edge/src/platform-linux.cc Fri Jul 31 23:11:53 2009 +++ /branches/bleeding_edge/src/platform-linux.cc Fri Sep 4 04:16:51 2009 @@ -56,6 +56,8 @@ #include "v8.h" #include "platform.h" +#include "top.h" +#include "v8threads.h" namespace v8 { @@ -580,6 +582,7 @@ #ifdef ENABLE_LOGGING_AND_PROFILING static Sampler* active_sampler_ = NULL; +static pthread_t vm_thread_ = 0; #if !defined(__GLIBC__) && (defined(__arm__) || defined(__thumb__)) @@ -606,6 +609,30 @@ enum ArmRegisters {R15 = 15, R13 = 13, R11 = 11}; #endif + + +// A function that determines if a signal handler is called in the context +// of a VM thread. +// +// The problem is that SIGPROF signal can be delivered to an arbitrary thread +// (see http://code.google.com/p/google-perftools/issues/detail?id=106#c2) +// So, if the signal is being handled in the context of a non-VM thread, +// it means that the VM thread is running, and trying to sample its stack can +// cause a crash. +static inline bool IsVmThread() { + // In the case of a single VM thread, this check is enough. + if (pthread_equal(pthread_self(), vm_thread_)) return true; + // If there are multiple threads that use VM, they must have a thread id + // stored in TLS. To verify that the thread is really executing VM, + // we check Top's data. Having that ThreadManager::RestoreThread first + // restores ThreadLocalTop from TLS, and only then erases the TLS value, + // reading Top::thread_id() should not be affected by races. + if (ThreadManager::HasId() && !ThreadManager::IsArchived() && + ThreadManager::CurrentId() == Top::thread_id()) { + return true; + } + return false; +} static void ProfilerSignalHandler(int signal, siginfo_t* info, void* context) { @@ -640,7 +667,8 @@ sample.fp = mcontext.arm_fp; #endif #endif - active_sampler_->SampleStack(&sample); + if (IsVmThread()) + active_sampler_->SampleStack(&sample); } // We always sample the VM state. @@ -678,6 +706,8 @@ // platforms. if (active_sampler_ != NULL) return; + vm_thread_ = pthread_self(); + // Request profiling signals. struct sigaction sa; sa.sa_sigaction = ProfilerSignalHandler; @@ -712,6 +742,7 @@ active_sampler_ = NULL; active_ = false; } + #endif // ENABLE_LOGGING_AND_PROFILING ======================================= --- /branches/bleeding_edge/src/v8threads.cc Wed Aug 19 08:14:11 2009 +++ /branches/bleeding_edge/src/v8threads.cc Fri Sep 4 04:16:51 2009 @@ -241,7 +241,10 @@ } -int ThreadManager::next_id_ = 0; +// Thread ids must start with 1, because in TLS having thread id 0 can't +// be distinguished from not having a thread id at all (since NULL is +// defined as 0.) +int ThreadManager::last_id_ = 0; Mutex* ThreadManager::mutex_ = OS::CreateMutex(); ThreadHandle ThreadManager::mutex_owner_(ThreadHandle::INVALID); ThreadHandle ThreadManager::lazily_archived_thread_(ThreadHandle::INVALID); @@ -250,7 +253,7 @@ void ThreadManager::ArchiveThread() { ASSERT(!lazily_archived_thread_.IsValid()); - ASSERT(Thread::GetThreadLocal(thread_state_key) == NULL); + ASSERT(!IsArchived()); ThreadState* state = ThreadState::GetFree(); state->Unlink(); Thread::SetThreadLocal(thread_state_key, reinterpret_cast<void*>(state)); @@ -279,6 +282,11 @@ lazily_archived_thread_.Initialize(ThreadHandle::INVALID); lazily_archived_thread_state_ = NULL; } + + +bool ThreadManager::IsArchived() { + return Thread::HasThreadLocal(thread_state_key); +} void ThreadManager::Iterate(ObjectVisitor* v) { @@ -321,13 +329,19 @@ void ThreadManager::AssignId() { - if (!Thread::HasThreadLocal(thread_id_key)) { + if (!HasId()) { ASSERT(Locker::IsLocked()); - int thread_id = next_id_++; + int thread_id = ++last_id_; + ASSERT(thread_id > 0); // see the comment near last_id_ definition. Thread::SetThreadLocalInt(thread_id_key, thread_id); Top::set_thread_id(thread_id); } } + + +bool ThreadManager::HasId() { + return Thread::HasThreadLocal(thread_id_key); +} void ThreadManager::TerminateExecution(int thread_id) { ======================================= --- /branches/bleeding_edge/src/v8threads.h Wed Aug 19 08:14:11 2009 +++ /branches/bleeding_edge/src/v8threads.h Fri Sep 4 04:16:51 2009 @@ -86,6 +86,7 @@ static void ArchiveThread(); static bool RestoreThread(); + static bool IsArchived(); static void Iterate(ObjectVisitor* v); static void MarkCompactPrologue(bool is_compacting); @@ -94,6 +95,7 @@ static int CurrentId(); static void AssignId(); + static bool HasId(); static void TerminateExecution(int thread_id); @@ -101,7 +103,7 @@ private: static void EagerlyArchiveThread(); - static int next_id_; // V8 threads are identified through an integer. + static int last_id_; // V8 threads are identified through an integer. static Mutex* mutex_; static ThreadHandle mutex_owner_; static ThreadHandle lazily_archived_thread_; ======================================= --- /branches/bleeding_edge/test/cctest/test-log.cc Mon Aug 24 20:18:40 2009 +++ /branches/bleeding_edge/test/cctest/test-log.cc Fri Sep 4 04:16:51 2009 @@ -5,12 +5,15 @@ #ifdef ENABLE_LOGGING_AND_PROFILING #ifdef __linux__ +#include <math.h> +#include <pthread.h> #include <signal.h> #include <unistd.h> -#endif +#endif // __linux__ #include "v8.h" #include "log.h" +#include "v8threads.h" #include "cctest.h" using v8::internal::Address; @@ -155,9 +158,10 @@ #ifdef __linux__ struct sigaction old_sigprof_handler; +pthread_t our_thread; static void SigProfSignalHandler(int signal, siginfo_t* info, void* context) { - if (signal != SIGPROF) return; + if (signal != SIGPROF || !pthread_equal(pthread_self(), our_thread)) return; was_sigprof_received = true; old_sigprof_handler.sa_sigaction(signal, info, context); } @@ -185,6 +189,7 @@ // Intercept SIGPROF handler to make sure that the test process // had received it. Under load, system can defer it causing test failure. // It is important to execute this after 'ResumeProfiler'. + our_thread = pthread_self(); was_sigprof_received = false; struct sigaction sa; sa.sa_sigaction = SigProfSignalHandler; @@ -278,6 +283,158 @@ i::FLAG_prof = saved_prof; i::FLAG_prof_auto = saved_prof_auto; } + + +// Profiling multiple threads that use V8 is currently only available on Linux. +#ifdef __linux__ + +namespace { + +class LoopingThread : public v8::internal::Thread { + public: + LoopingThread() + : v8::internal::Thread(), + semaphore_(v8::internal::OS::CreateSemaphore(0)), + run_(true) { + } + + virtual ~LoopingThread() { delete semaphore_; } + + void Run() { + self_ = pthread_self(); + RunLoop(); + } + + void SendSigProf() { pthread_kill(self_, SIGPROF); } + + void Stop() { run_ = false; } + + bool WaitForRunning() { return semaphore_->Wait(1000000); } + + protected: + bool IsRunning() { return run_; } + + virtual void RunLoop() = 0; + + void SetV8ThreadId() { + v8_thread_id_ = v8::V8::GetCurrentThreadId(); + } + + void SignalRunning() { semaphore_->Signal(); } + + private: + v8::internal::Semaphore* semaphore_; + bool run_; + pthread_t self_; + int v8_thread_id_; +}; + + +class LoopingJsThread : public LoopingThread { + public: + void RunLoop() { + { + v8::Locker locker; + CHECK(v8::internal::ThreadManager::HasId()); + SetV8ThreadId(); + } + while (IsRunning()) { + v8::Locker locker; + v8::HandleScope scope; + v8::Persistent<v8::Context> context = v8::Context::New(); + v8::Context::Scope context_scope(context); + SignalRunning(); + CompileAndRunScript( + "var j; for (var i=0; i<10000; ++i) { j = Math.sin(i); }"); + context.Dispose(); + i::OS::Sleep(1); + } + } +}; + + +class LoopingNonJsThread : public LoopingThread { + public: + void RunLoop() { + v8::Locker locker; + v8::Unlocker unlocker; + // Now thread has V8's id, but will not run VM code. + CHECK(v8::internal::ThreadManager::HasId()); + double i = 10; + SignalRunning(); + while (IsRunning()) { + i = sin(i); + i::OS::Sleep(1); + } + } +}; + + +class TestSampler : public v8::internal::Sampler { + public: + TestSampler() + : Sampler(0, true), + semaphore_(v8::internal::OS::CreateSemaphore(0)), + was_sample_stack_called_(false) { + } + + ~TestSampler() { delete semaphore_; } + + void SampleStack(v8::internal::TickSample*) { + was_sample_stack_called_ = true; + } + + void Tick(v8::internal::TickSample*) { semaphore_->Signal(); } + + bool WaitForTick() { return semaphore_->Wait(1000000); } + + void Reset() { was_sample_stack_called_ = false; } + + bool WasSampleStackCalled() { return was_sample_stack_called_; } + + private: + v8::internal::Semaphore* semaphore_; + bool was_sample_stack_called_; +}; + + +} // namespace + +TEST(ProfMultipleThreads) { + // V8 needs to be initialized before the first Locker + // instantiation. Otherwise, Top::Initialize will reset + // thread_id_ in ThreadTopLocal. + v8::HandleScope scope; + v8::Handle<v8::Context> env = v8::Context::New(); + env->Enter(); + + LoopingJsThread jsThread; + jsThread.Start(); + LoopingNonJsThread nonJsThread; + nonJsThread.Start(); + + TestSampler sampler; + sampler.Start(); + CHECK(!sampler.WasSampleStackCalled()); + jsThread.WaitForRunning(); + jsThread.SendSigProf(); + CHECK(sampler.WaitForTick()); + CHECK(sampler.WasSampleStackCalled()); + sampler.Reset(); + CHECK(!sampler.WasSampleStackCalled()); + nonJsThread.WaitForRunning(); + nonJsThread.SendSigProf(); + CHECK(sampler.WaitForTick()); + CHECK(!sampler.WasSampleStackCalled()); + sampler.Stop(); + + jsThread.Stop(); + nonJsThread.Stop(); + jsThread.Join(); + nonJsThread.Join(); +} + +#endif // __linux__ static inline bool IsStringEqualTo(const char* r, const char* s) { --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
