Fix data races and leaks related to v8::Lockers

BUG=v8:3618
R=ishell@chromium.org, svenpanne@chromium.org
LOG=n

Review URL: https://codereview.chromium.org/637263002

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

include/v8.h

@@ -5801,8 +5801,6 @@ class V8_EXPORT Locker {
   bool top_level_;
   internal::Isolate* isolate_;
 
-  static bool active_;
-
   // Disallow copying and assigning.
   Locker(const Locker&);
   void operator=(const Locker&);

src/isolate.h

@@ -179,7 +179,12 @@ typedef ZoneList<Handle<Object> > ZoneObjectList;
 class ThreadId {
  public:
   // Creates an invalid ThreadId.
-  ThreadId() : id_(kInvalidId) {}
+  ThreadId() { base::NoBarrier_Store(&id_, kInvalidId); }
+
+  ThreadId& operator=(const ThreadId& other) {
+    base::NoBarrier_Store(&id_, base::NoBarrier_Load(&other.id_));
+    return *this;
+  }
 
   // Returns ThreadId for current thread.
   static ThreadId Current() { return ThreadId(GetCurrentThreadId()); }
@@ -189,17 +194,17 @@ class ThreadId {
 
   // Compares ThreadIds for equality.
   INLINE(bool Equals(const ThreadId& other) const) {
-    return id_ == other.id_;
+    return base::NoBarrier_Load(&id_) == base::NoBarrier_Load(&other.id_);
   }
 
   // Checks whether this ThreadId refers to any thread.
   INLINE(bool IsValid() const) {
-    return id_ != kInvalidId;
+    return base::NoBarrier_Load(&id_) != kInvalidId;
   }
 
   // Converts ThreadId to an integer representation
   // (required for public API: V8::V8::GetCurrentThreadId).
-  int ToInteger() const { return id_; }
+  int ToInteger() const { return static_cast<int>(base::NoBarrier_Load(&id_)); }
 
   // Converts ThreadId to an integer representation
   // (required for public API: V8::V8::TerminateExecution).
@@ -208,13 +213,13 @@ class ThreadId {
  private:
   static const int kInvalidId = -1;
 
-  explicit ThreadId(int id) : id_(id) {}
+  explicit ThreadId(int id) { base::NoBarrier_Store(&id_, id); }
 
   static int AllocateThreadId();
 
   static int GetCurrentThreadId();
 
-  int id_;
+  base::Atomic32 id_;
 
   static base::Atomic32 highest_thread_id_;
 

src/v8threads.cc

@@ -14,9 +14,13 @@
 namespace v8 {
 
 
+namespace {
+
 // Track whether this V8 instance has ever called v8::Locker. This allows the
 // API code to verify that the lock is always held when V8 is being entered.
-bool Locker::active_ = false;
+base::Atomic32 g_locker_was_ever_used_ = 0;
+
+}  // namespace
 
 
 // Once the Locker is initialized, the current thread will be guaranteed to have
@@ -27,7 +31,7 @@ void Locker::Initialize(v8::Isolate* isolate) {
   top_level_ = true;
   isolate_ = reinterpret_cast<i::Isolate*>(isolate);
   // Record that the Locker has been used at least once.
-  active_ = true;
+  base::NoBarrier_Store(&g_locker_was_ever_used_, 1);
   // Get the big lock if necessary.
   if (!isolate_->thread_manager()->IsLockedByCurrentThread()) {
     isolate_->thread_manager()->Lock();
@@ -64,7 +68,7 @@ bool Locker::IsLocked(v8::Isolate* isolate) {
 
 
 bool Locker::IsActive() {
-  return active_;
+  return !!base::NoBarrier_Load(&g_locker_was_ever_used_);
 }
 
 

test/cctest/test-lockers.cc

@@ -141,6 +141,7 @@ class JoinableThread {
 
   void Join() {
     semaphore_.Wait();
+    thread_.Join();
   }
 
   virtual void Run() = 0;