Author: [email protected]
Date: Fri Mar  6 03:03:14 2009
New Revision: 1436

Modified:
    branches/bleeding_edge/src/debug.cc
    branches/bleeding_edge/src/debug.h
    branches/bleeding_edge/src/execution.cc
    branches/bleeding_edge/src/runtime.cc
    branches/bleeding_edge/src/top.cc
    branches/bleeding_edge/src/top.h
    branches/bleeding_edge/test/cctest/test-debug.cc

Log:
All preemption requests are now ignored while in the debugger. This ensures  
that no change of V8 thread happenes  while in the debugger. The only thing  
that happens is that a flag is set to indicate that preemption happened.  
When the debugger is left preemption is requested if it occourred while in  
the debugger.

Moved the debugger related global variables from Top to thread local in  
Debug.
Review URL: http://codereview.chromium.org/39124

Modified: branches/bleeding_edge/src/debug.cc
==============================================================================
--- branches/bleeding_edge/src/debug.cc (original)
+++ branches/bleeding_edge/src/debug.cc Fri Mar  6 03:03:14 2009
@@ -400,12 +400,17 @@

  // 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 @@
  }


+// 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 @@
            *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::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 @@
    // 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 @@
  }


+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::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"),

Modified: branches/bleeding_edge/src/debug.h
==============================================================================
--- branches/bleeding_edge/src/debug.h  (original)
+++ branches/bleeding_edge/src/debug.h  Fri Mar  6 03:03:14 2009
@@ -36,10 +36,16 @@
  #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 @@
    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 @@
    // 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 @@
    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 @@
    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 @@

      // 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 @@
  // 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 @@

    ~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 @@
    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.

Modified: branches/bleeding_edge/src/execution.cc
==============================================================================
--- branches/bleeding_edge/src/execution.cc     (original)
+++ branches/bleeding_edge/src/execution.cc     Fri Mar  6 03:03:14 2009
@@ -523,7 +523,12 @@

    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();
    }

Modified: branches/bleeding_edge/src/runtime.cc
==============================================================================
--- branches/bleeding_edge/src/runtime.cc       (original)
+++ branches/bleeding_edge/src/runtime.cc       Fri Mar  6 03:03:14 2009
@@ -4967,7 +4967,7 @@
    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 @@

    // 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 @@
    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();

Modified: branches/bleeding_edge/src/top.cc
==============================================================================
--- branches/bleeding_edge/src/top.cc   (original)
+++ branches/bleeding_edge/src/top.cc   Fri Mar  6 03:03:14 2009
@@ -38,9 +38,6 @@

  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 @@

    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.
@@ -292,44 +285,6 @@
  void Top::UnregisterTryCatchHandler(v8::TryCatch* that) {
    ASSERT(thread_local_.try_catch_handler_ == that);
    thread_local_.try_catch_handler_ = that->next_;
-}
-
-
-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_;
  }



Modified: branches/bleeding_edge/src/top.h
==============================================================================
--- branches/bleeding_edge/src/top.h    (original)
+++ branches/bleeding_edge/src/top.h    Fri Mar  6 03:03:14 2009
@@ -182,13 +182,6 @@
    // 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 @@
    // 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 @@
  // 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.

Modified: branches/bleeding_edge/test/cctest/test-debug.cc
==============================================================================
--- branches/bleeding_edge/test/cctest/test-debug.cc    (original)
+++ branches/bleeding_edge/test/cctest/test-debug.cc    Fri Mar  6 03:03:14  
2009
@@ -511,7 +511,7 @@
                                           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 @@
                                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 @@
                                 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 @@
                                         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 @@
                             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 @@
                                     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 @@
      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 @@
                              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 @@
    // The host dispatch callback should be called.
    CHECK_EQ(1, host_dispatch_hit_count);
  }
-

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to