[v8-dev] [v8] r5035 committed - Fix concurrent access to VMState::current_state_....

[codesite-noreply]

Revision: 5035
Author: mikhail.naga...@gmail.com
Date: Thu Jul  8 01:00:38 2010
Log: Fix concurrent access to VMState::current_state_.
The main fix is for current_state() and external_callback() accessors.
I also applied memory access ordering on current_state_ modification,
mainly to reflect the fact that it is being shared among VM and profiler
sampler threads.

BUG=361

Review URL: http://codereview.chromium.org/2852047
http://code.google.com/p/v8/source/detail?r=5035

Modified:
 /branches/bleeding_edge/src/vm-state-inl.h
 /branches/bleeding_edge/src/vm-state.cc
 /branches/bleeding_edge/src/vm-state.h

=======================================
--- /branches/bleeding_edge/src/vm-state-inl.h  Thu Apr  8 06:37:39 2010
+++ /branches/bleeding_edge/src/vm-state-inl.h  Thu Jul  8 01:00:38 2010
@@ -74,8 +74,10 @@
   if (state == EXTERNAL) state = OTHER;
 #endif
   state_ = state;
-  previous_ = current_state_;  // Save the previous state.
-  current_state_ = this;       // Install the new state.
+  // Save the previous state.
+  previous_ = reinterpret_cast<VMState*>(current_state_);
+  // Install the new state.
+  OS::ReleaseStore(&current_state_, reinterpret_cast<AtomicWord>(this));

 #ifdef ENABLE_LOGGING_AND_PROFILING
   if (FLAG_log_state_changes) {
@@ -103,7 +105,8 @@

 VMState::~VMState() {
   if (disabled_) return;
-  current_state_ = previous_;  // Return to the previous state.
+  // Return to the previous state.
+  OS::ReleaseStore(&current_state_,  
reinterpret_cast<AtomicWord>(previous_));

 #ifdef ENABLE_LOGGING_AND_PROFILING
   if (FLAG_log_state_changes) {
=======================================
--- /branches/bleeding_edge/src/vm-state.cc     Thu Apr  8 06:37:39 2010
+++ /branches/bleeding_edge/src/vm-state.cc     Thu Jul  8 01:00:38 2010
@@ -33,7 +33,7 @@
 namespace internal {

 #ifdef ENABLE_VMSTATE_TRACKING
-VMState* VMState::current_state_ = NULL;
+AtomicWord VMState::current_state_ = 0;
 #endif

 } }  // namespace v8::internal
=======================================
--- /branches/bleeding_edge/src/vm-state.h      Thu Apr  8 07:00:51 2010
+++ /branches/bleeding_edge/src/vm-state.h      Thu Jul  8 01:00:38 2010
@@ -44,15 +44,17 @@

   // Used for debug asserts.
   static bool is_outermost_external() {
-    return current_state_ == NULL;
+    return current_state_ == 0;
   }

   static StateTag current_state() {
-    return current_state_ ? current_state_->state() : EXTERNAL;
+    VMState* state = reinterpret_cast<VMState*>(current_state_);
+    return state ? state->state() : EXTERNAL;
   }

   static Address external_callback() {
-    return current_state_ ? current_state_->external_callback_ : NULL;
+    VMState* state = reinterpret_cast<VMState*>(current_state_);
+    return state ? state->external_callback_ : NULL;
   }

  private:
@@ -62,7 +64,7 @@
   Address external_callback_;

   // A stack of VM states.
-  static VMState* current_state_;
+  static AtomicWord current_state_;
 #else
  public:
   explicit VMState(StateTag state) {}

--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev