Reviewers: jarin,
Description:
[d8] Fix tsan bugs
script_executed and last_run are read/written by multiple threads. Also
externalized_shared_contents_ is modified by multiple threads.
BUG=4306
[email protected]
LOG=n
Please review this at https://codereview.chromium.org/1252623003/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+30, -20 lines):
M src/d8.h
M src/d8.cc
M test/mjsunit/mjsunit.status
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);
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);
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);
result = RunMain(isolate, argc, argv);
}
#endif
Index: src/d8.h
diff --git a/src/d8.h b/src/d8.h
index
4e217640af1b13c63d6470ec497daaa9ae1c6887..c7a43db059e4ce33a8f8b56276161678e3ccbe60
100644
--- a/src/d8.h
+++ b/src/d8.h
@@ -293,9 +293,7 @@ class Worker {
class ShellOptions {
public:
ShellOptions()
- : script_executed(false),
- last_run(true),
- send_idle_notification(false),
+ : send_idle_notification(false),
invoke_weak_callbacks(false),
omit_quit(false),
stress_opt(false),
@@ -310,18 +308,30 @@ class ShellOptions {
isolate_sources(NULL),
icu_data_file(NULL),
natives_blob(NULL),
- snapshot_blob(NULL) {}
+ snapshot_blob(NULL),
+ script_executed_(false),
+ last_run_(true) {}
~ShellOptions() {
delete[] isolate_sources;
}
bool use_interactive_shell() {
- return (interactive_shell || !script_executed) && !test_shell;
+ return (interactive_shell || !script_executed()) && !test_shell;
+ }
+
+ bool script_executed() { return base::NoBarrier_Load(&script_executed_);
}
+
+ void set_script_executed(bool value) {
+ return base::NoBarrier_Store(&script_executed_, value);
+ }
+
+ bool last_run() { return base::NoBarrier_Load(&last_run_); }
+
+ void set_last_run(bool value) {
+ return base::NoBarrier_Store(&last_run_, value);
}
- bool script_executed;
- bool last_run;
bool send_idle_notification;
bool invoke_weak_callbacks;
bool omit_quit;
@@ -338,6 +348,10 @@ class ShellOptions {
const char* icu_data_file;
const char* natives_blob;
const char* snapshot_blob;
+
+ private:
+ base::Atomic32 script_executed_;
+ base::Atomic32 last_run_;
};
#ifdef V8_SHARED
Index: test/mjsunit/mjsunit.status
diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
index
7dca71898659bf879d6a1293cd2f842e3bc9b689..9a3b16457e13c34f79cc4ccd1b471300587207e3
100644
--- a/test/mjsunit/mjsunit.status
+++ b/test/mjsunit/mjsunit.status
@@ -249,9 +249,6 @@
# 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:4306)
- 'd8-worker-sharedarraybuffer': [SKIP],
}], # ALWAYS
['novfp3 == True', {
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
