[v8-dev] Fix concurrent access to VMState::current_state_. (issue2852047)

[mnaganov]

Reviewers: Vitaly,

Description:
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

Please review this at http://codereview.chromium.org/2852047/show

Affected files:
  M src/vm-state-inl.h
  M src/vm-state.h
  M src/vm-state.cc


Index: src/vm-state-inl.h
diff --git a/src/vm-state-inl.h b/src/vm-state-inl.h
index  
4df2cfda73b4f47cc584287afffa986d26f91d1d..aa4cedbb134b532e20020f160ba8930a827b3c9d  
100644
--- a/src/vm-state-inl.h
+++ b/src/vm-state-inl.h
@@ -74,8 +74,10 @@ VMState::VMState(StateTag state)
   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(StateTag state)

 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) {
Index: src/vm-state.cc
diff --git a/src/vm-state.cc b/src/vm-state.cc
index  
3859efb82484c87cc3d0ad7ed25b500715745568..6bd737dfd639d7dcbb99cfc40235c8d2ff515000  
100644
--- a/src/vm-state.cc
+++ b/src/vm-state.cc
@@ -33,7 +33,7 @@ namespace v8 {
 namespace internal {

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

 } }  // namespace v8::internal
Index: src/vm-state.h
diff --git a/src/vm-state.h b/src/vm-state.h
index  
241df4c9d48cbb36ce5f84378f7594ab831cbc5c..080eb8ded6a2b3fc4325528ccf8dfb59fff5c70e  
100644
--- a/src/vm-state.h
+++ b/src/vm-state.h
@@ -44,15 +44,17 @@ class VMState BASE_EMBEDDED {

   // 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 @@ class VMState BASE_EMBEDDED {
   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