Commit 053b8434 authored by binji's avatar binji Committed by Commit bot

[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
R=jarin@chromium.org
LOG=n

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

Cr-Commit-Position: refs/heads/master@{#29912}
parent a87db3de
...@@ -312,7 +312,6 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source, ...@@ -312,7 +312,6 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source,
bool report_exceptions, SourceType source_type) { bool report_exceptions, SourceType source_type) {
HandleScope handle_scope(isolate); HandleScope handle_scope(isolate);
TryCatch try_catch(isolate); TryCatch try_catch(isolate);
options.script_executed = true;
MaybeLocal<Value> maybe_result; MaybeLocal<Value> maybe_result;
{ {
...@@ -1499,7 +1498,7 @@ void SourceGroup::ExecuteInThread() { ...@@ -1499,7 +1498,7 @@ void SourceGroup::ExecuteInThread() {
Isolate::CreateParams create_params; Isolate::CreateParams create_params;
create_params.array_buffer_allocator = Shell::array_buffer_allocator; create_params.array_buffer_allocator = Shell::array_buffer_allocator;
Isolate* isolate = Isolate::New(create_params); Isolate* isolate = Isolate::New(create_params);
do { for (int i = 0; i < Shell::options.stress_runs; ++i) {
next_semaphore_.Wait(); next_semaphore_.Wait();
{ {
Isolate::Scope iscope(isolate); Isolate::Scope iscope(isolate);
...@@ -1516,7 +1515,7 @@ void SourceGroup::ExecuteInThread() { ...@@ -1516,7 +1515,7 @@ void SourceGroup::ExecuteInThread() {
Shell::CollectGarbage(isolate); Shell::CollectGarbage(isolate);
} }
done_semaphore_.Signal(); done_semaphore_.Signal();
} while (!Shell::options.last_run); }
isolate->Dispose(); isolate->Dispose();
} }
...@@ -1533,11 +1532,13 @@ void SourceGroup::StartExecuteInThread() { ...@@ -1533,11 +1532,13 @@ void SourceGroup::StartExecuteInThread() {
void SourceGroup::WaitForThread() { void SourceGroup::WaitForThread() {
if (thread_ == NULL) return; if (thread_ == NULL) return;
if (Shell::options.last_run) { done_semaphore_.Wait();
thread_->Join(); }
} else {
done_semaphore_.Wait();
} void SourceGroup::JoinThread() {
if (thread_ == NULL) return;
thread_->Join();
} }
...@@ -1931,6 +1932,11 @@ bool Shell::SetOptions(int argc, char* argv[]) { ...@@ -1931,6 +1932,11 @@ bool Shell::SetOptions(int argc, char* argv[]) {
enable_harmony_modules = true; enable_harmony_modules = true;
} else if (strncmp(argv[i], "--", 2) == 0) { } else if (strncmp(argv[i], "--", 2) == 0) {
printf("Warning: unknown flag %s.\nTry --help for options\n", argv[i]); printf("Warning: unknown flag %s.\nTry --help for options\n", argv[i]);
} else if (strcmp(str, "-e") == 0 && i + 1 < argc) {
options.script_executed = true;
} else if (strncmp(str, "-", 1) != 0) {
// Not a flag, so it must be a script to execute.
options.script_executed = true;
} }
} }
current->End(argc); current->End(argc);
...@@ -1947,7 +1953,7 @@ bool Shell::SetOptions(int argc, char* argv[]) { ...@@ -1947,7 +1953,7 @@ bool Shell::SetOptions(int argc, char* argv[]) {
} }
int Shell::RunMain(Isolate* isolate, int argc, char* argv[]) { int Shell::RunMain(Isolate* isolate, int argc, char* argv[], bool last_run) {
#ifndef V8_SHARED #ifndef V8_SHARED
for (int i = 1; i < options.num_isolates; ++i) { for (int i = 1; i < options.num_isolates; ++i) {
options.isolate_sources[i].StartExecuteInThread(); options.isolate_sources[i].StartExecuteInThread();
...@@ -1956,7 +1962,7 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[]) { ...@@ -1956,7 +1962,7 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[]) {
{ {
HandleScope scope(isolate); HandleScope scope(isolate);
Local<Context> context = CreateEvaluationContext(isolate); Local<Context> context = CreateEvaluationContext(isolate);
if (options.last_run && options.use_interactive_shell()) { if (last_run && options.use_interactive_shell()) {
// Keep using the same context in the interactive shell. // Keep using the same context in the interactive shell.
evaluation_context_.Reset(isolate, context); evaluation_context_.Reset(isolate, context);
} }
...@@ -1969,7 +1975,11 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[]) { ...@@ -1969,7 +1975,11 @@ int Shell::RunMain(Isolate* isolate, int argc, char* argv[]) {
CollectGarbage(isolate); CollectGarbage(isolate);
#ifndef V8_SHARED #ifndef V8_SHARED
for (int i = 1; i < options.num_isolates; ++i) { for (int i = 1; i < options.num_isolates; ++i) {
options.isolate_sources[i].WaitForThread(); if (last_run) {
options.isolate_sources[i].JoinThread();
} else {
options.isolate_sources[i].WaitForThread();
}
} }
CleanupWorkers(); CleanupWorkers();
#endif // !V8_SHARED #endif // !V8_SHARED
...@@ -2095,6 +2105,7 @@ bool Shell::SerializeValue(Isolate* isolate, Local<Value> value, ...@@ -2095,6 +2105,7 @@ bool Shell::SerializeValue(Isolate* isolate, Local<Value> value,
contents = sab->GetContents(); contents = sab->GetContents();
} else { } else {
contents = sab->Externalize(); contents = sab->Externalize();
base::LockGuard<base::Mutex> lock_guard(workers_mutex_.Pointer());
externalized_shared_contents_.Add(contents); externalized_shared_contents_.Add(contents);
} }
out_data->WriteSharedArrayBufferContents(contents); out_data->WriteSharedArrayBufferContents(contents);
...@@ -2250,10 +2261,8 @@ void Shell::CleanupWorkers() { ...@@ -2250,10 +2261,8 @@ void Shell::CleanupWorkers() {
} }
// Now that all workers are terminated, we can re-enable Worker creation. // Now that all workers are terminated, we can re-enable Worker creation.
{ base::LockGuard<base::Mutex> lock_guard(workers_mutex_.Pointer());
base::LockGuard<base::Mutex> lock_guard(workers_mutex_.Pointer()); allow_new_workers_ = true;
allow_new_workers_ = true;
}
for (int i = 0; i < externalized_shared_contents_.length(); ++i) { for (int i = 0; i < externalized_shared_contents_.length(); ++i) {
const SharedArrayBuffer::Contents& contents = const SharedArrayBuffer::Contents& contents =
...@@ -2395,26 +2404,29 @@ int Shell::Main(int argc, char* argv[]) { ...@@ -2395,26 +2404,29 @@ int Shell::Main(int argc, char* argv[]) {
Testing::SetStressRunType(options.stress_opt Testing::SetStressRunType(options.stress_opt
? Testing::kStressTypeOpt ? Testing::kStressTypeOpt
: Testing::kStressTypeDeopt); : Testing::kStressTypeDeopt);
int stress_runs = Testing::GetStressRuns(); options.stress_runs = Testing::GetStressRuns();
for (int i = 0; i < stress_runs && result == 0; i++) { for (int i = 0; i < options.stress_runs && result == 0; i++) {
printf("============ Stress %d/%d ============\n", i + 1, stress_runs); printf("============ Stress %d/%d ============\n", i + 1,
options.stress_runs);
Testing::PrepareStressRun(i); Testing::PrepareStressRun(i);
options.last_run = (i == stress_runs - 1); bool last_run = i == options.stress_runs - 1;
result = RunMain(isolate, argc, argv); result = RunMain(isolate, argc, argv, last_run);
} }
printf("======== Full Deoptimization =======\n"); printf("======== Full Deoptimization =======\n");
Testing::DeoptimizeAll(); Testing::DeoptimizeAll();
#if !defined(V8_SHARED) #if !defined(V8_SHARED)
} else if (i::FLAG_stress_runs > 0) { } else if (i::FLAG_stress_runs > 0) {
int stress_runs = i::FLAG_stress_runs; options.stress_runs = i::FLAG_stress_runs;
for (int i = 0; i < stress_runs && result == 0; i++) { for (int i = 0; i < options.stress_runs && result == 0; i++) {
printf("============ Run %d/%d ============\n", i + 1, stress_runs); printf("============ Run %d/%d ============\n", i + 1,
options.last_run = (i == stress_runs - 1); options.stress_runs);
result = RunMain(isolate, argc, argv); bool last_run = i == options.stress_runs - 1;
result = RunMain(isolate, argc, argv, last_run);
} }
#endif #endif
} else { } else {
result = RunMain(isolate, argc, argv); bool last_run = true;
result = RunMain(isolate, argc, argv, last_run);
} }
// Run interactive shell if explicitly requested or if no script has been // Run interactive shell if explicitly requested or if no script has been
......
...@@ -119,6 +119,7 @@ class SourceGroup { ...@@ -119,6 +119,7 @@ class SourceGroup {
#ifndef V8_SHARED #ifndef V8_SHARED
void StartExecuteInThread(); void StartExecuteInThread();
void WaitForThread(); void WaitForThread();
void JoinThread();
private: private:
class IsolateThread : public base::Thread { class IsolateThread : public base::Thread {
...@@ -273,12 +274,12 @@ class ShellOptions { ...@@ -273,12 +274,12 @@ class ShellOptions {
public: public:
ShellOptions() ShellOptions()
: script_executed(false), : script_executed(false),
last_run(true),
send_idle_notification(false), send_idle_notification(false),
invoke_weak_callbacks(false), invoke_weak_callbacks(false),
omit_quit(false), omit_quit(false),
stress_opt(false), stress_opt(false),
stress_deopt(false), stress_deopt(false),
stress_runs(1),
interactive_shell(false), interactive_shell(false),
test_shell(false), test_shell(false),
dump_heap_constants(false), dump_heap_constants(false),
...@@ -300,12 +301,12 @@ class ShellOptions { ...@@ -300,12 +301,12 @@ class ShellOptions {
} }
bool script_executed; bool script_executed;
bool last_run;
bool send_idle_notification; bool send_idle_notification;
bool invoke_weak_callbacks; bool invoke_weak_callbacks;
bool omit_quit; bool omit_quit;
bool stress_opt; bool stress_opt;
bool stress_deopt; bool stress_deopt;
int stress_runs;
bool interactive_shell; bool interactive_shell;
bool test_shell; bool test_shell;
bool dump_heap_constants; bool dump_heap_constants;
...@@ -340,7 +341,7 @@ class Shell : public i::AllStatic { ...@@ -340,7 +341,7 @@ class Shell : public i::AllStatic {
static void ReportException(Isolate* isolate, TryCatch* try_catch); static void ReportException(Isolate* isolate, TryCatch* try_catch);
static Local<String> ReadFile(Isolate* isolate, const char* name); static Local<String> ReadFile(Isolate* isolate, const char* name);
static Local<Context> CreateEvaluationContext(Isolate* isolate); static Local<Context> CreateEvaluationContext(Isolate* isolate);
static int RunMain(Isolate* isolate, int argc, char* argv[]); static int RunMain(Isolate* isolate, int argc, char* argv[], bool last_run);
static int Main(int argc, char* argv[]); static int Main(int argc, char* argv[]);
static void Exit(int exit_code); static void Exit(int exit_code);
static void OnExit(Isolate* isolate); static void OnExit(Isolate* isolate);
......
...@@ -252,9 +252,6 @@ ...@@ -252,9 +252,6 @@
# BUG(chromium:508074). Remove this once the issue is fixed. # BUG(chromium:508074). Remove this once the issue is fixed.
'harmony/arrow-rest-params': [PASS, NO_VARIANTS], 'harmony/arrow-rest-params': [PASS, NO_VARIANTS],
'harmony/rest-params': [PASS, ['no_snap == True', NO_VARIANTS]], 'harmony/rest-params': [PASS, ['no_snap == True', NO_VARIANTS]],
# BUG(v8:4306)
'd8-worker-sharedarraybuffer': [SKIP],
}], # ALWAYS }], # ALWAYS
['novfp3 == True', { ['novfp3 == True', {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment