[d8] Fix tsan bugs

src/d8.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 
 // Defined when linking against shared lib on Windows.
 #if defined(USING_V8_SHARED) && !defined(V8_SHARED)
 #define V8_SHARED
 #endif
 
@@ skipping 317 matching lines @@
 bool Shell::ExecuteString(Isolate* isolate, Local<String> source,
                           Local<Value> name, bool print_result,
                           bool report_exceptions, SourceType source_type) {
 #ifndef V8_SHARED
   bool FLAG_debugger = i::FLAG_debugger;
 #else
   bool FLAG_debugger = false;
 #endif  // !V8_SHARED
   HandleScope handle_scope(isolate);
   TryCatch try_catch(isolate);
-  options.script_executed = true;
+  options.set_script_executed(true);

[Jarin, 2015/07/23 06:49:14]

It looks like the set_script_executed could be calculated from the command line
arguments: script_executed is true if there is an argument not prefixed with '-'
or if there is an '-e' argument. Perhaps we could just do a walk over the
arguments (basically cut-down version of SourceGroup::Execute) rather than the
Atomic hackery?

[binji, 2015/07/24 23:13:08]

Done.

   if (FLAG_debugger) {
     // When debugging make exceptions appear to be uncaught.
     try_catch.SetVerbose(true);
   }
 
   MaybeLocal<Value> maybe_result;
   {
     PerIsolateData* data = PerIsolateData::Get(isolate);
     Local<Context> realm =
         Local<Context>::New(isolate, data->realms_[data->realm_current_]);
@@ skipping 1289 matching lines @@
         Local<Context> context = Shell::CreateEvaluationContext(isolate);
         {
           Context::Scope cscope(context);
           PerIsolateData::RealmScope realm_scope(PerIsolateData::Get(isolate));
           Execute(isolate);
         }
       }
       Shell::CollectGarbage(isolate);
     }
     done_semaphore_.Signal();
-  } while (!Shell::options.last_run);
+  } while (!Shell::options.last_run());
 
   isolate->Dispose();
 }
 
 
 void SourceGroup::StartExecuteInThread() {
   if (thread_ == NULL) {
     thread_ = new IsolateThread(this);
     thread_->Start();
   }
   next_semaphore_.Signal();
 }
 
 
 void SourceGroup::WaitForThread() {
   if (thread_ == NULL) return;
-  if (Shell::options.last_run) {
+  if (Shell::options.last_run()) {
     thread_->Join();
   } else {
     done_semaphore_.Wait();
   }
 }
 
 
 SerializationData::~SerializationData() {
   // Any ArrayBuffer::Contents are owned by this SerializationData object if
   // ownership hasn't been transferred out via ReadArrayBufferContents.
@@ skipping 417 matching lines @@
 
 int Shell::RunMain(Isolate* isolate, int argc, char* argv[]) {
 #ifndef V8_SHARED
   for (int i = 1; i < options.num_isolates; ++i) {
     options.isolate_sources[i].StartExecuteInThread();
   }
 #endif  // !V8_SHARED
   {
     HandleScope scope(isolate);
     Local<Context> context = CreateEvaluationContext(isolate);
-    if (options.last_run && options.use_interactive_shell()) {
+    if (options.last_run() && options.use_interactive_shell()) {
       // Keep using the same context in the interactive shell.
       evaluation_context_.Reset(isolate, context);
 #ifndef V8_SHARED
       // If the interactive debugger is enabled make sure to activate
       // it before running the files passed on the command line.
       if (i::FLAG_debugger) {
         InstallUtilityScript(isolate);
       }
 #endif  // !V8_SHARED
     }
@@ skipping 125 matching lines @@
     if (!FindInObjectList(sab, to_transfer)) {
       Throw(isolate, "SharedArrayBuffer must be transferred");
       return false;
     }
 
     SharedArrayBuffer::Contents contents;
     if (sab->IsExternal()) {
       contents = sab->GetContents();
     } else {
       contents = sab->Externalize();
+      base::LockGuard<base::Mutex> lock_guard(workers_mutex_.Pointer());
       externalized_shared_contents_.Add(contents);
     }
     out_data->WriteSharedArrayBufferContents(contents);
   } else if (value->IsObject()) {
     Local<Object> object = Local<Object>::Cast(value);
     if (FindInObjectList(object, *seen_objects)) {
       Throw(isolate, "Duplicated objects not supported");
       return false;
     }
     seen_objects->Add(object);
@@ skipping 135 matching lines @@
     workers_.Clear();
   }
 
   for (int i = 0; i < workers_copy.length(); ++i) {
     Worker* worker = workers_copy[i];
     worker->WaitForThread();
     delete worker;
   }
 
   // Now that all workers are terminated, we can re-enable Worker creation.
-  {
-    base::LockGuard<base::Mutex> lock_guard(workers_mutex_.Pointer());
-    allow_new_workers_ = true;
-  }
+  base::LockGuard<base::Mutex> lock_guard(workers_mutex_.Pointer());
+  allow_new_workers_ = true;
 
   for (int i = 0; i < externalized_shared_contents_.length(); ++i) {
     const SharedArrayBuffer::Contents& contents =
         externalized_shared_contents_[i];
     Shell::array_buffer_allocator->Free(contents.Data(), contents.ByteLength());
   }
   externalized_shared_contents_.Clear();
 }
 
 
@@ skipping 127 matching lines @@
 #endif
 
     if (options.stress_opt || options.stress_deopt) {
       Testing::SetStressRunType(options.stress_opt
                                 ? Testing::kStressTypeOpt
                                 : Testing::kStressTypeDeopt);
       int stress_runs = Testing::GetStressRuns();
       for (int i = 0; i < stress_runs && result == 0; i++) {
         printf("============ Stress %d/%d ============\n", i + 1, stress_runs);
         Testing::PrepareStressRun(i);
-        options.last_run = (i == stress_runs - 1);
+        options.set_last_run(i == stress_runs - 1);

[Jarin, 2015/07/23 06:49:14]

I am a bit confused about how this can be run in parallel with anything. I
thought that everything should be stopped at this point, no?

[binji, 2015/07/24 23:13:08]

No, the SourceGroup spawns a thread per isolate that persists across RunMain.
But I refactored my change so that there is no need for last_run anymore.

         result = RunMain(isolate, argc, argv);
       }
       printf("======== Full Deoptimization =======\n");
       Testing::DeoptimizeAll();
 #if !defined(V8_SHARED)
     } else if (i::FLAG_stress_runs > 0) {
       int stress_runs = i::FLAG_stress_runs;
       for (int i = 0; i < stress_runs && result == 0; i++) {
         printf("============ Run %d/%d ============\n", i + 1, stress_runs);
-        options.last_run = (i == stress_runs - 1);
+        options.set_last_run(i == stress_runs - 1);

[Jarin, 2015/07/23 06:49:14]

Same here.

         result = RunMain(isolate, argc, argv);
       }
 #endif
     } else {
       result = RunMain(isolate, argc, argv);
     }
 
     // Run interactive shell if explicitly requested or if no script has been
     // executed, but never on --test
     if (options.use_interactive_shell()) {
@@ skipping 30 matching lines @@
 }
 
 }  // namespace v8
 
 
 #ifndef GOOGLE3
 int main(int argc, char* argv[]) {
   return v8::Shell::Main(argc, argv);
 }
 #endif