From: mikhail.naganov@gmail.com Date: Fri, 10 Jun 2011 09:54:04 +0000 (+0000) Subject: "Deiceolate" Thread classes. X-Git-Tag: upstream/4.7.83~19171 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=508b22c436cf9b0837e948dbb407d3aa25f21725;p=platform%2Fupstream%2Fv8.git "Deiceolate" Thread classes. Thread class was receiving an isolate parameter by default. This approact violates the assumption that only VM threads can have an associated isolate, and can lead to troubles, because accessing the same isolate from different threads leads to race conditions. This was found by investigating mysterious failures of the CPU profiler layout test on Linux Chromium. As almost all threads were associated with some isolate, the sampler was trying to sample them. As a side effect, we have also fixed the DebuggerAgent test. Thanks to Vitaly for help in fixing isolates handling! R=vitalyr@chromium.org BUG=none TEST=none git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8259 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/include/v8.h b/include/v8.h index 443e064..b6e73eb 100644 --- a/include/v8.h +++ b/include/v8.h @@ -3098,8 +3098,10 @@ class V8EXPORT V8 { * because of a call to TerminateExecution. In that case there are * still JavaScript frames on the stack and the termination * exception is still active. + * + * \param isolate The isolate in which to check. */ - static bool IsExecutionTerminating(); + static bool IsExecutionTerminating(Isolate* isolate = NULL); /** * Releases any resources used by v8 and stops any utility threads diff --git a/samples/shell.cc b/samples/shell.cc index e253fba..950370a 100644 --- a/samples/shell.cc +++ b/samples/shell.cc @@ -170,7 +170,7 @@ class SourceGroup { class IsolateThread : public v8::internal::Thread { public: explicit IsolateThread(SourceGroup* group) - : v8::internal::Thread(NULL, GetThreadOptions()), group_(group) {} + : v8::internal::Thread(GetThreadOptions()), group_(group) {} virtual void Run() { group_->ExecuteInThread(); diff --git a/src/api.cc b/src/api.cc index 91e4618..ec1cb6f 100644 --- a/src/api.cc +++ b/src/api.cc @@ -4902,9 +4902,10 @@ void V8::TerminateExecution(Isolate* isolate) { } -bool V8::IsExecutionTerminating() { - i::Isolate* isolate = i::Isolate::Current(); - return IsExecutionTerminatingCheck(isolate); +bool V8::IsExecutionTerminating(Isolate* isolate) { + i::Isolate* i_isolate = isolate != NULL ? + reinterpret_cast(isolate) : i::Isolate::Current(); + return IsExecutionTerminatingCheck(i_isolate); } diff --git a/src/cpu-profiler.cc b/src/cpu-profiler.cc index f54e3e8..2b62212 100644 --- a/src/cpu-profiler.cc +++ b/src/cpu-profiler.cc @@ -46,9 +46,8 @@ static const int kTickSamplesBufferChunkSize = 64*KB; static const int kTickSamplesBufferChunksCount = 16; -ProfilerEventsProcessor::ProfilerEventsProcessor(Isolate* isolate, - ProfileGenerator* generator) - : Thread(isolate, "v8:ProfEvntProc"), +ProfilerEventsProcessor::ProfilerEventsProcessor(ProfileGenerator* generator) + : Thread("v8:ProfEvntProc"), generator_(generator), running_(true), ticks_buffer_(sizeof(TickSampleEventRecord), @@ -507,7 +506,7 @@ void CpuProfiler::StartProcessorIfNotStarted() { saved_logging_nesting_ = isolate->logger()->logging_nesting_; isolate->logger()->logging_nesting_ = 0; generator_ = new ProfileGenerator(profiles_); - processor_ = new ProfilerEventsProcessor(isolate, generator_); + processor_ = new ProfilerEventsProcessor(generator_); NoBarrier_Store(&is_profiling_, true); processor_->Start(); // Enumerate stuff we already have in the heap. diff --git a/src/cpu-profiler.h b/src/cpu-profiler.h index f9f6167..ad4bc2e 100644 --- a/src/cpu-profiler.h +++ b/src/cpu-profiler.h @@ -134,8 +134,7 @@ class TickSampleEventRecord BASE_EMBEDDED { // methods called by event producers: VM and stack sampler threads. class ProfilerEventsProcessor : public Thread { public: - ProfilerEventsProcessor(Isolate* isolate, - ProfileGenerator* generator); + explicit ProfilerEventsProcessor(ProfileGenerator* generator); virtual ~ProfilerEventsProcessor() {} // Thread control. diff --git a/src/debug-agent.cc b/src/debug-agent.cc index 498b88a..520bc62 100644 --- a/src/debug-agent.cc +++ b/src/debug-agent.cc @@ -116,8 +116,8 @@ void DebuggerAgent::CreateSession(Socket* client) { } // Create a new session and hook up the debug message handler. - session_ = new DebuggerAgentSession(isolate(), this, client); - v8::Debug::SetMessageHandler2(DebuggerAgentMessageHandler); + session_ = new DebuggerAgentSession(this, client); + isolate_->debugger()->SetMessageHandler(DebuggerAgentMessageHandler); session_->Start(); } @@ -203,7 +203,9 @@ void DebuggerAgentSession::Run() { // Send the request received to the debugger. v8::Debug::SendCommand(reinterpret_cast(temp.start()), - len); + len, + NULL, + reinterpret_cast(agent_->isolate())); if (is_closing_session) { // Session is closed. diff --git a/src/debug-agent.h b/src/debug-agent.h index a25002e..e167871 100644 --- a/src/debug-agent.h +++ b/src/debug-agent.h @@ -43,24 +43,27 @@ class DebuggerAgentSession; // handles connection from a remote debugger. class DebuggerAgent: public Thread { public: - DebuggerAgent(Isolate* isolate, const char* name, int port) - : Thread(isolate, name), + DebuggerAgent(const char* name, int port) + : Thread(name), + isolate_(Isolate::Current()), name_(StrDup(name)), port_(port), server_(OS::CreateSocket()), terminate_(false), session_access_(OS::CreateMutex()), session_(NULL), terminate_now_(OS::CreateSemaphore(0)), listening_(OS::CreateSemaphore(0)) { - ASSERT(Isolate::Current()->debugger_agent_instance() == NULL); - Isolate::Current()->set_debugger_agent_instance(this); + ASSERT(isolate_->debugger_agent_instance() == NULL); + isolate_->set_debugger_agent_instance(this); } ~DebuggerAgent() { - Isolate::Current()->set_debugger_agent_instance(NULL); + isolate_->set_debugger_agent_instance(NULL); delete server_; } void Shutdown(); void WaitUntilListening(); + Isolate* isolate() { return isolate_; } + private: void Run(); void CreateSession(Socket* socket); @@ -68,6 +71,7 @@ class DebuggerAgent: public Thread { void CloseSession(); void OnSessionClosed(DebuggerAgentSession* session); + Isolate* isolate_; SmartPointer name_; // Name of the embedding application. int port_; // Port to use for the agent. Socket* server_; // Server socket for listen/accept. @@ -88,8 +92,8 @@ class DebuggerAgent: public Thread { // debugger and sends debugger events/responses to the remote debugger. class DebuggerAgentSession: public Thread { public: - DebuggerAgentSession(Isolate* isolate, DebuggerAgent* agent, Socket* client) - : Thread(isolate, "v8:DbgAgntSessn"), + DebuggerAgentSession(DebuggerAgent* agent, Socket* client) + : Thread("v8:DbgAgntSessn"), agent_(agent), client_(client) {} void DebuggerMessage(Vector message); diff --git a/src/debug.cc b/src/debug.cc index 85c4b5e..f341fc6 100644 --- a/src/debug.cc +++ b/src/debug.cc @@ -796,7 +796,6 @@ bool Debug::Load() { // Return if debugger is already loaded. if (IsLoaded()) return true; - ASSERT(Isolate::Current() == isolate_); Debugger* debugger = isolate_->debugger(); // Bail out if we're already in the process of compiling the native @@ -1048,7 +1047,6 @@ Handle Debug::CheckBreakPoints(Handle break_point_objects) { // Check whether a single break point object is triggered. bool Debug::CheckBreakPoint(Handle break_point_object) { - ASSERT(Isolate::Current() == isolate_); Factory* factory = isolate_->factory(); HandleScope scope(isolate_); @@ -1234,7 +1232,6 @@ bool Debug::IsBreakOnException(ExceptionBreakType type) { void Debug::PrepareStep(StepAction step_action, int step_count) { - ASSERT(Isolate::Current() == isolate_); HandleScope scope(isolate_); ASSERT(Debug::InDebugger()); @@ -1739,7 +1736,6 @@ void Debug::RemoveDebugInfo(Handle debug_info) { void Debug::SetAfterBreakTarget(JavaScriptFrame* frame) { - ASSERT(Isolate::Current() == isolate_); HandleScope scope(isolate_); // Get the executing function in which the debug break occurred. @@ -1872,7 +1868,6 @@ bool Debug::IsDebugGlobal(GlobalObject* global) { void Debug::ClearMirrorCache() { - ASSERT(Isolate::Current() == isolate_); PostponeInterruptsScope postpone(isolate_); HandleScope scope(isolate_); ASSERT(isolate_->context() == *Debug::debug_context()); @@ -1892,7 +1887,6 @@ void Debug::ClearMirrorCache() { void Debug::CreateScriptCache() { - ASSERT(Isolate::Current() == isolate_); Heap* heap = isolate_->heap(); HandleScope scope(isolate_); @@ -1934,7 +1928,6 @@ void Debug::AddScriptToScriptCache(Handle