From d4d470326668a69a3d00cae51ae96d375f862d95 Mon Sep 17 00:00:00 2001
From: ahaas <ahaas@chromium.org>
Date: Wed, 22 Jun 2016 04:28:19 -0700
Subject: [PATCH] [wasm] Move the semaphore for parallel compilation to the
 wasm module.

If the semaphore is stored as a local variable in {CompileInParallel},
then the semaphore was sometimes deallocated too early and caused
the compilation tasks to crash. This only happens with libc-2.19,
libc-2.21 fixes the problem.

R=mlippautz@chromium.org, rossberg@chromium.org

Review-Url: https://codereview.chromium.org/2080223006
Cr-Commit-Position: refs/heads/master@{#37183}
---
 src/base/platform/semaphore.cc |  3 +++
 src/wasm/wasm-module.cc        | 12 ++++++------
 src/wasm/wasm-module.h         |  8 ++++++++
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/base/platform/semaphore.cc b/src/base/platform/semaphore.cc
index 7c86f43b32..7bf598662a 100644
--- a/src/base/platform/semaphore.cc
+++ b/src/base/platform/semaphore.cc
@@ -101,6 +101,9 @@ Semaphore::~Semaphore() {
 
 void Semaphore::Signal() {
   int result = sem_post(&native_handle_);
+  // This check may fail with <libc-2.21, which we use on the try bots, if the
+  // semaphore is destroyed while sem_post is still executed. A work around is
+  // to extend the lifetime of the semaphore.
   CHECK_EQ(0, result);
 }
 
diff --git a/src/wasm/wasm-module.cc b/src/wasm/wasm-module.cc
index b1d9cb0213..6d9b06b2b5 100644
--- a/src/wasm/wasm-module.cc
+++ b/src/wasm/wasm-module.cc
@@ -348,7 +348,8 @@ WasmModule::WasmModule()
       start_function_index(-1),
       origin(kWasmOrigin),
       globals_size(0),
-      indirect_table_size(0) {}
+      indirect_table_size(0),
+      pending_tasks(new base::Semaphore(0)) {}
 
 static MaybeHandle<JSFunction> ReportFFIError(ErrorThrower& thrower,
                                               const char* error, uint32_t index,
@@ -614,16 +615,15 @@ void CompileInParallel(Isolate* isolate, const WasmModule* module,
                                 *module_env, *thrower);
 
   // Objects for the synchronization with the background threads.
-  base::SmartPointer<base::Semaphore> pending_tasks(new base::Semaphore(0));
   base::Mutex result_mutex;
   base::AtomicNumber<size_t> next_unit(
       static_cast<size_t>(FLAG_skip_compiling_wasm_funcs));
 
   // 2) The main thread spawns {WasmCompilationTask} instances which run on
   //    the background threads.
-  base::SmartArrayPointer<uint32_t> task_ids(
-      StartCompilationTasks(isolate, compilation_units, executed_units,
-                            pending_tasks.get(), result_mutex, next_unit));
+  base::SmartArrayPointer<uint32_t> task_ids(StartCompilationTasks(
+      isolate, compilation_units, executed_units, module->pending_tasks.get(),
+      result_mutex, next_unit));
 
   // 3.a) The background threads and the main thread pick one compilation
   //      unit at a time and execute the parallel phase of the compilation
@@ -640,7 +640,7 @@ void CompileInParallel(Isolate* isolate, const WasmModule* module,
   }
   // 4) After the parallel phase of all compilation units has started, the
   //    main thread waits for all {WasmCompilationTask} instances to finish.
-  WaitForCompilationTasks(isolate, task_ids.get(), pending_tasks.get());
+  WaitForCompilationTasks(isolate, task_ids.get(), module->pending_tasks.get());
   // Finish the compilation of the remaining compilation units.
   FinishCompilationUnits(executed_units, functions, result_mutex);
 }
diff --git a/src/wasm/wasm-module.h b/src/wasm/wasm-module.h
index 180f5bcde3..3329b69b5c 100644
--- a/src/wasm/wasm-module.h
+++ b/src/wasm/wasm-module.h
@@ -178,6 +178,14 @@ struct WasmModule {
   std::vector<uint16_t> function_table;        // function table.
   std::vector<WasmImport> import_table;        // import table.
   std::vector<WasmExport> export_table;        // export table.
+  // We store the semaphore here to extend its lifetime. In <libc-2.21, which we
+  // use on the try bots, semaphore::Wait() can return while some compilation
+  // tasks are still executing semaphore::Signal(). If the semaphore is cleaned
+  // up right after semaphore::Wait() returns, then this can cause an
+  // invalid-semaphore error in the compilation tasks.
+  // TODO(wasm): Move this semaphore back to CompileInParallel when the try bots
+  // switch to libc-2.21 or higher.
+  base::SmartPointer<base::Semaphore> pending_tasks;
 
   WasmModule();