Reland of d8 workers: make sure Shell::Quit is only called once (patchset #1 id:1...
authorbinji <binji@chromium.org>
Tue, 14 Jul 2015 23:04:18 +0000 (16:04 -0700)
committerCommit bot <commit-bot@chromium.org>
Tue, 14 Jul 2015 23:04:32 +0000 (23:04 +0000)
Reason for revert:
Looks like the failure is unrelated to my change (still fails after the revert). See http://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Win/builds/856/steps/webkit_unit_tests/logs/stdio

Original issue's description:
> Revert of d8 workers: make sure Shell::Quit is only called once (patchset #5 id:80001 of https://codereview.chromium.org/1230403003/)
>
> Reason for revert:
> Breaks webkit_unit_tests. See http://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Win/builds/853/steps/webkit_unit_tests/logs/stdio
>
> Original issue's description:
> > d8 workers: make sure Shell::Quit is only called once
> >
> > When running with isolates, Quit can be called simultaneously by two threads.
> > If this happens, then both threads try to clean up the Workers, which could
> > crash.
> >
> > BUG=v8:4279
> > R=jarin@chromium.org
> > R=machenbach@chromium.org
> > LOG=n
> >
> > Committed: https://crrev.com/76184292b392d107609f21662a949b58bb1e258c
> > Cr-Commit-Position: refs/heads/master@{#29654}
>
> TBR=jarin@chromium.org,machenbach@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=v8:4279
>
> Committed: https://crrev.com/6b2c6eb75678747afca59b4a78ace597e218145d
> Cr-Commit-Position: refs/heads/master@{#29656}

TBR=jarin@chromium.org,machenbach@chromium.org,adamk@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4279

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

Cr-Commit-Position: refs/heads/master@{#29663}

src/d8.cc
src/d8.h
test/mjsunit/mjsunit.status

index 988a31a..19aa86b 100644 (file)
--- a/src/d8.cc
+++ b/src/d8.cc
@@ -216,6 +216,7 @@ Persistent<Context> Shell::evaluation_context_;
 ArrayBuffer::Allocator* Shell::array_buffer_allocator;
 ShellOptions Shell::options;
 const char* Shell::kPrompt = "d8> ";
+base::OnceType Shell::quit_once_ = V8_ONCE_INIT;
 
 #ifndef V8_SHARED
 bool CounterMap::Match(void* key1, void* key2) {
@@ -809,16 +810,22 @@ void Shell::WorkerTerminate(const v8::FunctionCallbackInfo<v8::Value>& args) {
 #endif  // !V8_SHARED
 
 
-void Shell::Quit(const v8::FunctionCallbackInfo<v8::Value>& args) {
-  int exit_code = args[0]->Int32Value();
+void Shell::QuitOnce(v8::FunctionCallbackInfo<v8::Value>* args) {
+  int exit_code = (*args)[0]->Int32Value();
 #ifndef V8_SHARED
   CleanupWorkers();
 #endif  // !V8_SHARED
-  OnExit(args.GetIsolate());
+  OnExit(args->GetIsolate());
   exit(exit_code);
 }
 
 
+void Shell::Quit(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  base::CallOnce(&quit_once_, &QuitOnce,
+                 const_cast<v8::FunctionCallbackInfo<v8::Value>*>(&args));
+}
+
+
 void Shell::Version(const v8::FunctionCallbackInfo<v8::Value>& args) {
   args.GetReturnValue().Set(
       String::NewFromUtf8(args.GetIsolate(), V8::GetVersion()));
index 393392e..ff83544 100644 (file)
--- a/src/d8.h
+++ b/src/d8.h
@@ -15,6 +15,9 @@
 #include "src/base/compiler-specific.h"
 #endif  // !V8_SHARED
 
+#include "src/base/once.h"
+
+
 namespace v8 {
 
 
@@ -410,6 +413,7 @@ class Shell : public i::AllStatic {
 
   static void Print(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Write(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void QuitOnce(v8::FunctionCallbackInfo<v8::Value>* args);
   static void Quit(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Version(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Read(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -467,6 +471,7 @@ class Shell : public i::AllStatic {
 
  private:
   static Persistent<Context> evaluation_context_;
+  static base::OnceType quit_once_;
 #ifndef V8_SHARED
   static Persistent<Context> utility_context_;
   static CounterMap* counter_map_;
index 864cd3e..0b25f4c 100644 (file)
   # BUG(chromium:508074). Remove this once the issue is fixed.
   'harmony/arrow-rest-params': [PASS, NO_VARIANTS],
   'harmony/rest-params': [PASS, ['no_snap == True', NO_VARIANTS]],
-
-  # BUG(v8:4279).
-  'regress/regress-4279': [PASS, ['isolates', SKIP]],
 }],  # ALWAYS
 
 ['novfp3 == True', {