[d8] Fix tsan bugs

src/d8.cc

Index: src/d8.cc
diff --git a/src/d8.cc b/src/d8.cc
index a56d029f5a5fb2fedb3fb568578ca9b90f53bada..1dad5694970cb343e88b5138dac20d86ba775ef2 100644
--- a/src/d8.cc
+++ b/src/d8.cc
@@ -335,7 +335,7 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source,
 #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);
@@ -1645,7 +1645,7 @@ void SourceGroup::ExecuteInThread() {
       Shell::CollectGarbage(isolate);
     }
     done_semaphore_.Signal();
-  } while (!Shell::options.last_run);
+  } while (!Shell::options.last_run());
 
   isolate->Dispose();
 }
@@ -1662,7 +1662,7 @@ void SourceGroup::StartExecuteInThread() {
 
 void SourceGroup::WaitForThread() {
   if (thread_ == NULL) return;
-  if (Shell::options.last_run) {
+  if (Shell::options.last_run()) {
     thread_->Join();
   } else {
     done_semaphore_.Wait();
@@ -2100,7 +2100,7 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[]) {
   {
     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
@@ -2246,6 +2246,7 @@ bool Shell::SerializeValue(Isolate* isolate, Local<Value> value,
       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);
@@ -2401,10 +2402,8 @@ void Shell::CleanupWorkers() {
   }
 
   // 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 =
@@ -2552,7 +2551,7 @@ int Shell::Main(int argc, char* argv[]) {
       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");
@@ -2562,7 +2561,7 @@ int Shell::Main(int argc, char* argv[]) {
       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