All preemption requests are now ignored while in the debugger. This ensures that...
authorsgjesse@chromium.org <sgjesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 6 Mar 2009 11:03:14 +0000 (11:03 +0000)
committersgjesse@chromium.org <sgjesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 6 Mar 2009 11:03:14 +0000 (11:03 +0000)
Moved the debugger related global variables from Top to thread local in Debug.
Review URL: http://codereview.chromium.org/39124

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

src/debug.cc
src/debug.h
src/execution.cc
src/runtime.cc
src/top.cc
src/top.h
test/cctest/test-debug.cc

index 620739c..b704eed 100644 (file)
@@ -400,12 +400,17 @@ DebugInfoListNode* Debug::debug_info_list_ = NULL;
 
 // Threading support.
 void Debug::ThreadInit() {
+  thread_local_.break_count_ = 0;
+  thread_local_.break_id_ = 0;
+  thread_local_.break_frame_id_ = StackFrame::NO_ID;
   thread_local_.last_step_action_ = StepNone;
   thread_local_.last_statement_position_ = RelocInfo::kNoPosition;
   thread_local_.step_count_ = 0;
   thread_local_.last_fp_ = 0;
   thread_local_.step_into_fp_ = 0;
   thread_local_.after_break_target_ = 0;
+  thread_local_.debugger_entry_ = NULL;
+  thread_local_.preemption_pending_ = false;
 }
 
 
@@ -611,6 +616,13 @@ void Debug::Unload() {
 }
 
 
+// Set the flag indicating that preemption happened during debugging.
+void Debug::PreemptionWhileInDebugger() {
+  ASSERT(InDebugger());
+  Debug::set_preemption_pending(true);
+}
+
+
 void Debug::Iterate(ObjectVisitor* v) {
   v->VisitPointer(bit_cast<Object**, Code**>(&(debug_break_return_entry_)));
   v->VisitPointer(bit_cast<Object**, Code**>(&(debug_break_return_)));
@@ -743,7 +755,7 @@ bool Debug::CheckBreakPoint(Handle<Object> break_point_object) {
           *Factory::LookupAsciiSymbol("IsBreakPointTriggered"))));
 
   // Get the break id as an object.
-  Handle<Object> break_id = Factory::NewNumberFromInt(Top::break_id());
+  Handle<Object> break_id = Factory::NewNumberFromInt(Debug::break_id());
 
   // Call HandleBreakPointx.
   bool caught_exception = false;
@@ -873,7 +885,7 @@ void Debug::FloodWithOneShot(Handle<SharedFunctionInfo> shared) {
 
 void Debug::FloodHandlerWithOneShot() {
   // Iterate through the JavaScript stack looking for handlers.
-  StackFrame::Id id = Top::break_frame_id();
+  StackFrame::Id id = break_frame_id();
   if (id == StackFrame::NO_ID) {
     // If there is no JavaScript stack don't do anything.
     return;
@@ -913,7 +925,7 @@ void Debug::PrepareStep(StepAction step_action, int step_count) {
   // any. The debug frame will only be present if execution was stopped due to
   // hitting a break point. In other situations (e.g. unhandled exception) the
   // debug frame is not present.
-  StackFrame::Id id = Top::break_frame_id();
+  StackFrame::Id id = break_frame_id();
   if (id == StackFrame::NO_ID) {
     // If there is no JavaScript stack don't do anything.
     return;
@@ -1118,6 +1130,18 @@ Handle<Object> Debug::GetSourceBreakLocations(
 }
 
 
+void Debug::NewBreak(StackFrame::Id break_frame_id) {
+  thread_local_.break_frame_id_ = break_frame_id;
+  thread_local_.break_id_ = ++thread_local_.break_count_;
+}
+
+
+void Debug::SetBreak(StackFrame::Id break_frame_id, int break_id) {
+  thread_local_.break_frame_id_ = break_frame_id;
+  thread_local_.break_id_ = break_id;
+}
+
+
 // Handle stepping into a function.
 void Debug::HandleStepIn(Handle<JSFunction> function,
                          Address fp,
@@ -1381,7 +1405,7 @@ Handle<Object> Debugger::MakeJSObject(Vector<const char> constructor_name,
 
 Handle<Object> Debugger::MakeExecutionState(bool* caught_exception) {
   // Create the execution state object.
-  Handle<Object> break_id = Factory::NewNumberFromInt(Top::break_id());
+  Handle<Object> break_id = Factory::NewNumberFromInt(Debug::break_id());
   const int argc = 1;
   Object** argv[argc] = { break_id.location() };
   return MakeJSObject(CStrVector("MakeExecutionState"),
index f7760d0..41193b4 100644 (file)
 #include "factory.h"
 #include "platform.h"
 #include "string-stream.h"
+#include "v8threads.h"
 
 
 namespace v8 { namespace internal {
 
+
+// Forward declarations.
+class EnterDebugger;
+
+
 // Step actions. NOTE: These values are in macros.py as well.
 enum StepAction {
   StepNone = -1,  // Stepping not prepared.
@@ -166,7 +172,8 @@ class Debug {
   static bool Load();
   static void Unload();
   static bool IsLoaded() { return !debug_context_.is_null(); }
-  static bool InDebugger() { return Top::is_break(); }
+  static bool InDebugger() { return thread_local_.debugger_entry_ != NULL; }
+  static void PreemptionWhileInDebugger();
   static void Iterate(ObjectVisitor* v);
 
   static Object* Break(Arguments args);
@@ -211,6 +218,16 @@ class Debug {
   // Fast check to see if any break points are active.
   inline static bool has_break_points() { return has_break_points_; }
 
+  static void NewBreak(StackFrame::Id break_frame_id);
+  static void SetBreak(StackFrame::Id break_frame_id, int break_id);
+  static StackFrame::Id break_frame_id() {
+    return thread_local_.break_frame_id_;
+  }
+  static int break_id() { return thread_local_.break_id_; }
+
+
+
+
   static bool StepInActive() { return thread_local_.step_into_fp_ != 0; }
   static void HandleStepIn(Handle<JSFunction> function,
                            Address fp,
@@ -218,6 +235,20 @@ class Debug {
   static Address step_in_fp() { return thread_local_.step_into_fp_; }
   static Address* step_in_fp_addr() { return &thread_local_.step_into_fp_; }
 
+  static EnterDebugger* debugger_entry() {
+    return thread_local_.debugger_entry_;
+  }
+  static void set_debugger_entry(EnterDebugger* entry) {
+    thread_local_.debugger_entry_ = entry;
+  }
+
+  static bool preemption_pending() {
+    return thread_local_.preemption_pending_;
+  }
+  static void set_preemption_pending(bool preemption_pending) {
+    thread_local_.preemption_pending_ = preemption_pending;
+  }
+
   // Getter and setter for the disable break state.
   static bool disable_break() { return disable_break_; }
   static void set_disable_break(bool disable_break) {
@@ -313,9 +344,18 @@ class Debug {
   static bool break_on_exception_;
   static bool break_on_uncaught_exception_;
 
-  // Per-thread:
+  // Per-thread data.
   class ThreadLocal {
    public:
+    // Counter for generating next break id.
+    int break_count_;
+
+    // Current break id.
+    int break_id_;
+
+    // Frame id for the frame of the current break.
+    StackFrame::Id break_frame_id_;
+
     // Step action for last step performed.
     StepAction last_step_action_;
 
@@ -333,6 +373,12 @@ class Debug {
 
     // Storage location for jump when exiting debug break calls.
     Address after_break_target_;
+
+    // Top debugger entry.
+    EnterDebugger* debugger_entry_;
+
+    // Preemption happened while debugging.
+    bool preemption_pending_;
   };
 
   // Storage location for registers when handling debug break calls
@@ -519,17 +565,34 @@ class DebugMessageThread: public Thread {
 // some reason could not be entered FailedToEnter will return true.
 class EnterDebugger BASE_EMBEDDED {
  public:
-  EnterDebugger() : has_js_frames_(!it_.done()) {
+  EnterDebugger()
+      : prev_(Debug::debugger_entry()),
+        has_js_frames_(!it_.done()) {
+    ASSERT(!Debug::preemption_pending());
+
+    // Link recursive debugger entry.
+    Debug::set_debugger_entry(this);
+
+    // If a preemption is pending when first entering the debugger clear it as
+    // we don't want preemption happening while executing JavaScript in the
+    // debugger. When recursively entering the debugger the preemption flag
+    // cannot be set as this is disabled while in the debugger (see
+    // RuntimePreempt).
+    if (prev_ == NULL && StackGuard::IsPreempted()) {
+      StackGuard::Continue(PREEMPT);
+    }
+    ASSERT(!StackGuard::IsPreempted());
+
     // Store the previous break id and frame id.
-    break_id_ = Top::break_id();
-    break_frame_id_ = Top::break_frame_id();
+    break_id_ = Debug::break_id();
+    break_frame_id_ = Debug::break_frame_id();
 
     // Create the new break info. If there is no JavaScript frames there is no
     // break frame id.
     if (has_js_frames_) {
-      Top::new_break(it_.frame()->id());
+      Debug::NewBreak(it_.frame()->id());
     } else {
-      Top::new_break(StackFrame::NO_ID);
+      Debug::NewBreak(StackFrame::NO_ID);
     }
 
     // Make sure that debugger is loaded and enter the debugger context.
@@ -543,7 +606,18 @@ class EnterDebugger BASE_EMBEDDED {
 
   ~EnterDebugger() {
     // Restore to the previous break state.
-    Top::set_break(break_frame_id_, break_id_);
+    Debug::SetBreak(break_frame_id_, break_id_);
+
+    // Request preemption when leaving the last debugger entry and a preemption
+    // had been recorded while debugging. This is to avoid starvation in some
+    // debugging scenarios.
+    if (prev_ == NULL && Debug::preemption_pending()) {
+      StackGuard::Preempt();
+      Debug::set_preemption_pending(false);
+    }
+  
+    // Leaving this debugger entry.
+    Debug::set_debugger_entry(prev_);
   }
 
   // Check whether the debugger could be entered.
@@ -553,6 +627,7 @@ class EnterDebugger BASE_EMBEDDED {
   inline bool HasJavaScriptFrames() { return has_js_frames_; }
 
  private:
+  EnterDebugger* prev_;  // Previous debugger entry if entered recursively.
   JavaScriptFrameIterator it_;
   const bool has_js_frames_;  // Were there any JavaScript frames?
   StackFrame::Id break_frame_id_;  // Previous break frame id.
index f721cbd..419cc92 100644 (file)
@@ -523,7 +523,12 @@ static Object* RuntimePreempt() {
 
   ContextSwitcher::PreemptionReceived();
 
-  {
+  if (Debug::InDebugger()) {
+    // If currently in the debugger don't do any actual preemption but record
+    // that preemption occoured while in the debugger.
+    Debug::PreemptionWhileInDebugger();
+  } else {
+    // Perform preemption.
     v8::Unlocker unlocker;
     Thread::YieldCPU();
   }
index 2e4d872..89b6a52 100644 (file)
@@ -4967,7 +4967,7 @@ static Object* Runtime_CheckExecutionState(Arguments args) {
   ASSERT(args.length() >= 1);
   CONVERT_NUMBER_CHECKED(int, break_id, Int32, args[0]);
   // Check that the break id is valid.
-  if (Top::break_id() == 0 || break_id != Top::break_id()) {
+  if (Debug::break_id() == 0 || break_id != Debug::break_id()) {
     return Top::Throw(Heap::illegal_execution_state_symbol());
   }
 
@@ -4985,7 +4985,7 @@ static Object* Runtime_GetFrameCount(Arguments args) {
 
   // Count all frames which are relevant to debugging stack trace.
   int n = 0;
-  StackFrame::Id id = Top::break_frame_id();
+  StackFrame::Id id = Debug::break_frame_id();
   if (id == StackFrame::NO_ID) {
     // If there is no JavaScript stack frame count is 0.
     return Smi::FromInt(0);
@@ -5030,7 +5030,7 @@ static Object* Runtime_GetFrameDetails(Arguments args) {
   CONVERT_NUMBER_CHECKED(int, index, Int32, args[1]);
 
   // Find the relevant frame with the requested index.
-  StackFrame::Id id = Top::break_frame_id();
+  StackFrame::Id id = Debug::break_frame_id();
   if (id == StackFrame::NO_ID) {
     // If there are no JavaScript stack frames return undefined.
     return Heap::undefined_value();
index ed4cb42..05a3836 100644 (file)
@@ -38,9 +38,6 @@ namespace v8 { namespace internal {
 
 ThreadLocalTop Top::thread_local_;
 Mutex* Top::break_access_ = OS::CreateMutex();
-StackFrame::Id Top::break_frame_id_;
-int Top::break_count_;
-int Top::break_id_;
 
 NoAllocationStringAllocator* preallocated_message_space = NULL;
 
@@ -222,10 +219,6 @@ void Top::Initialize() {
 
   InitializeThreadLocal();
 
-  break_frame_id_ = StackFrame::NO_ID;
-  break_count_ = 0;
-  break_id_ = 0;
-
   // Only preallocate on the first initialization.
   if (FLAG_preallocate_message_memory && (preallocated_message_space == NULL)) {
     // Start the thread which will set aside some memory.
@@ -295,44 +288,6 @@ void Top::UnregisterTryCatchHandler(v8::TryCatch* that) {
 }
 
 
-void Top::new_break(StackFrame::Id break_frame_id) {
-  ExecutionAccess access;
-  break_frame_id_ = break_frame_id;
-  break_id_ = ++break_count_;
-}
-
-
-void Top::set_break(StackFrame::Id break_frame_id, int break_id) {
-  ExecutionAccess access;
-  break_frame_id_ = break_frame_id;
-  break_id_ = break_id;
-}
-
-
-bool Top::check_break(int break_id) {
-  ExecutionAccess access;
-  return break_id == break_id_;
-}
-
-
-bool Top::is_break() {
-  ExecutionAccess access;
-  return break_id_ != 0;
-}
-
-
-StackFrame::Id Top::break_frame_id() {
-  ExecutionAccess access;
-  return break_frame_id_;
-}
-
-
-int Top::break_id() {
-  ExecutionAccess access;
-  return break_id_;
-}
-
-
 void Top::MarkCompactPrologue(bool is_compacting) {
   MarkCompactPrologue(is_compacting, &thread_local_);
 }
index 26151bd..175a7f6 100644 (file)
--- a/src/top.h
+++ b/src/top.h
@@ -182,13 +182,6 @@ class Top {
   // Generated code scratch locations.
   static void* formal_count_address() { return &thread_local_.formal_count_; }
 
-  static void new_break(StackFrame::Id break_frame_id);
-  static void set_break(StackFrame::Id break_frame_id, int break_id);
-  static bool check_break(int break_id);
-  static bool is_break();
-  static StackFrame::Id break_frame_id();
-  static int break_id();
-
   static void MarkCompactPrologue(bool is_compacting);
   static void MarkCompactEpilogue(bool is_compacting);
   static void MarkCompactPrologue(bool is_compacting,
@@ -304,15 +297,6 @@ class Top {
   // Mutex for serializing access to break control structures.
   static Mutex* break_access_;
 
-  // ID of the frame where execution is stopped by debugger.
-  static StackFrame::Id break_frame_id_;
-
-  // Counter to create unique id for each debug break.
-  static int break_count_;
-
-  // Current debug break, 0 if running.
-  static int break_id_;
-
   friend class SaveContext;
   friend class AssertNoContextChange;
   friend class ExecutionAccess;
@@ -326,12 +310,12 @@ class Top {
 // versions of GCC. See V8 issue 122 for details.
 class SaveContext BASE_EMBEDDED {
  public:
-  SaveContext() :
-      context_(Top::context()),
+  SaveContext()
+      context_(Top::context()),
 #if __GNUC_VERSION__ >= 40100 && __GNUC_VERSION__ < 40300
-      dummy_(Top::context()),
+        dummy_(Top::context()),
 #endif
-      prev_(Top::save_context()) {
+        prev_(Top::save_context()) {
     Top::set_save_context(this);
 
     // If there is no JS frame under the current C frame, use the value 0.
index f8a2dd0..253ab44 100644 (file)
@@ -511,7 +511,7 @@ static void DebugEventBreakPointHitCount(v8::DebugEvent event,
                                          v8::Handle<v8::Object> event_data,
                                          v8::Handle<v8::Value> data) {
   // When hitting a debug event listener there must be a break set.
-  CHECK(v8::internal::Top::is_break());
+  CHECK(v8::internal::Debug::break_id() != 0);
 
   // Count the number of breaks.
   if (event == v8::Break) {
@@ -551,7 +551,7 @@ static void DebugEventCounter(v8::DebugEvent event,
                               v8::Handle<v8::Object> event_data,
                               v8::Handle<v8::Value> data) {
   // When hitting a debug event listener there must be a break set.
-  CHECK(v8::internal::Top::is_break());
+  CHECK(v8::internal::Debug::break_id() != 0);
 
   // Count the number of breaks.
   if (event == v8::Break) {
@@ -609,7 +609,7 @@ static void DebugEventEvaluate(v8::DebugEvent event,
                                v8::Handle<v8::Object> event_data,
                                v8::Handle<v8::Value> data) {
   // When hitting a debug event listener there must be a break set.
-  CHECK(v8::internal::Top::is_break());
+  CHECK(v8::internal::Debug::break_id() != 0);
 
   if (event == v8::Break) {
     for (int i = 0; checks[i].expr != NULL; i++) {
@@ -635,7 +635,7 @@ static void DebugEventRemoveBreakPoint(v8::DebugEvent event,
                                        v8::Handle<v8::Object> event_data,
                                        v8::Handle<v8::Value> data) {
   // When hitting a debug event listener there must be a break set.
-  CHECK(v8::internal::Top::is_break());
+  CHECK(v8::internal::Debug::break_id() != 0);
 
   if (event == v8::Break) {
     break_point_hit_count++;
@@ -653,7 +653,7 @@ static void DebugEventStep(v8::DebugEvent event,
                            v8::Handle<v8::Object> event_data,
                            v8::Handle<v8::Value> data) {
   // When hitting a debug event listener there must be a break set.
-  CHECK(v8::internal::Top::is_break());
+  CHECK(v8::internal::Debug::break_id() != 0);
 
   if (event == v8::Break) {
     break_point_hit_count++;
@@ -679,7 +679,7 @@ static void DebugEventStepSequence(v8::DebugEvent event,
                                    v8::Handle<v8::Object> event_data,
                                    v8::Handle<v8::Value> data) {
   // When hitting a debug event listener there must be a break set.
-  CHECK(v8::internal::Top::is_break());
+  CHECK(v8::internal::Debug::break_id() != 0);
 
   if (event == v8::Break || event == v8::Exception) {
     // Check that the current function is the expected.
@@ -709,7 +709,7 @@ static void DebugEventBreakPointCollectGarbage(
     v8::Handle<v8::Object> event_data,
     v8::Handle<v8::Value> data) {
   // When hitting a debug event listener there must be a break set.
-  CHECK(v8::internal::Top::is_break());
+  CHECK(v8::internal::Debug::break_id() != 0);
 
   // Perform a garbage collection when break point is hit and continue. Based
   // on the number of break points hit either scavenge or mark compact
@@ -734,7 +734,7 @@ static void DebugEventBreak(v8::DebugEvent event,
                             v8::Handle<v8::Object> event_data,
                             v8::Handle<v8::Value> data) {
   // When hitting a debug event listener there must be a break set.
-  CHECK(v8::internal::Top::is_break());
+  CHECK(v8::internal::Debug::break_id() != 0);
 
   if (event == v8::Break) {
     // Count the number of breaks.
@@ -3695,4 +3695,3 @@ TEST(DebuggerHostDispatch) {
   // The host dispatch callback should be called.
   CHECK_EQ(1, host_dispatch_hit_count);
 }
-