[wasm] Add threading tests for a shared WasmEngine.

R=clemensh@chromium.org
TEST=cctest/test-wasm-shared-engine
BUG=v8:7424

Change-Id: I32510f33fb88c2f1a79864e6033d3aa53ad9fe48
Reviewed-on: https://chromium-review.googlesource.com/1150149
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54727}

src/isolate.cc

@@ -2615,7 +2615,7 @@ void Isolate::Deinit() {
     optimizing_compile_dispatcher_ = nullptr;
   }
 
-  wasm_engine()->TearDown();
+  wasm_engine()->DeleteCompileJobsOnIsolate(this);
 
   heap_.mark_compact_collector()->EnsureSweepingCompleted();
   heap_.memory_allocator()->unmapper()->EnsureUnmappingCompleted();
@@ -2658,12 +2658,14 @@ void Isolate::Deinit() {
   delete compiler_dispatcher_;
   compiler_dispatcher_ = nullptr;
 
-  // This stops cancelable tasks (i.e. concurrent masking tasks)
+  // This stops cancelable tasks (i.e. concurrent marking tasks)
   cancelable_task_manager()->CancelAndWait();
 
   heap_.TearDown();
   logger_->TearDown();
 
+  wasm_engine_.reset();
+
   if (FLAG_embedded_builtins) {
     if (DefaultEmbeddedBlob() == nullptr && embedded_blob() != nullptr) {
       // We own the embedded blob. Free it.

src/wasm/wasm-code-manager.cc

@@ -872,8 +872,8 @@ std::unique_ptr<NativeModule> WasmCodeManager::NewNativeModule(
         isolate, can_request_more, &mem, this, std::move(module), env));
     TRACE_HEAP("New NativeModule %p: Mem: %" PRIuPTR ",+%zu\n", this, start,
                size);
-    AssignRanges(start, end, ret.get());
     base::LockGuard<base::Mutex> lock(&native_modules_mutex_);
+    AssignRanges(start, end, ret.get());
     native_modules_.emplace(ret.get());
     return ret;
   }

src/wasm/wasm-engine.cc

@@ -20,7 +20,7 @@ namespace wasm {
 WasmEngine::WasmEngine(std::unique_ptr<WasmCodeManager> code_manager)
     : code_manager_(std::move(code_manager)) {}
 
-WasmEngine::~WasmEngine() = default;
+WasmEngine::~WasmEngine() { TearDown(); }
 
 bool WasmEngine::SyncValidate(Isolate* isolate, const ModuleWireBytes& bytes) {
   // TODO(titzer): remove dependency on the isolate.
@@ -185,10 +185,12 @@ CodeTracer* WasmEngine::GetCodeTracer() {
 }
 
 void WasmEngine::Register(CancelableTaskManager* task_manager) {
+  base::LockGuard<base::Mutex> guard(&mutex_);
   task_managers_.emplace_back(task_manager);
 }
 
 void WasmEngine::Unregister(CancelableTaskManager* task_manager) {
+  base::LockGuard<base::Mutex> guard(&mutex_);
   task_managers_.remove(task_manager);
 }
 
@@ -224,14 +226,25 @@ void WasmEngine::AbortCompileJobsOnIsolate(Isolate* isolate) {
   for (auto* job : isolate_jobs) job->Abort();
 }
 
+void WasmEngine::DeleteCompileJobsOnIsolate(Isolate* isolate) {
+  for (auto it = jobs_.begin(); it != jobs_.end();) {
+    if (it->first->isolate() == isolate) {
+      it = jobs_.erase(it);
+    } else {
+      ++it;
+    }
+  }
+}
+
 void WasmEngine::TearDown() {
-  // Cancel all registered task managers.
+  // Cancel all registered task managers. Locking not required since no more
+  // Isolates that could register or unregister task managers exist.
   for (auto task_manager : task_managers_) {
     task_manager->CancelAndWait();
   }
 
-  // Cancel all AsyncCompileJobs.
-  jobs_.clear();
+  // All AsyncCompileJobs have been canceled.
+  DCHECK(jobs_.empty());
 }
 
 namespace {

src/wasm/wasm-engine.h

@@ -117,9 +117,12 @@ class V8_EXPORT_PRIVATE WasmEngine {
   // Cancel all AsyncCompileJobs that belong to the given Isolate. Their
   // deletion is delayed until all tasks accessing the AsyncCompileJob finish
   // their execution. This is used to clean-up the isolate to be reused.
-  void AbortCompileJobsOnIsolate(Isolate*);
+  void AbortCompileJobsOnIsolate(Isolate* isolate);
 
-  void TearDown();
+  // Deletes all AsyncCompileJobs that belong to the given Isolate. Similar to
+  // the above {AbortCompileJobsOnIsolate} but does not delay deletion because
+  // this is only used during tear-down of the Isolate.
+  void DeleteCompileJobsOnIsolate(Isolate* isolate);
 
   // Call on process start and exit.
   static void InitializeOncePerProcess();
@@ -134,6 +137,7 @@ class V8_EXPORT_PRIVATE WasmEngine {
       Isolate* isolate, std::unique_ptr<byte[]> bytes_copy, size_t length,
       Handle<Context> context,
       std::unique_ptr<CompilationResultResolver> resolver);
+  void TearDown();
 
   // We use an AsyncCompileJob as the key for itself so that we can delete the
   // job from the map when it is finished.
@@ -142,10 +146,6 @@ class V8_EXPORT_PRIVATE WasmEngine {
   WasmMemoryTracker memory_tracker_;
   AccountingAllocator allocator_;
 
-  // Contains all CancelableTaskManagers that run tasks that are dependent
-  // on the isolate.
-  std::list<CancelableTaskManager*> task_managers_;
-
   // This mutex protects all information which is mutated concurrently or
   // fields that are initialized lazily on the first access.
   base::Mutex mutex_;
@@ -153,6 +153,10 @@ class V8_EXPORT_PRIVATE WasmEngine {
   //////////////////////////////////////////////////////////////////////////////
   // Protected by {mutex_}:
 
+  // Contains all CancelableTaskManagers that run tasks that are dependent
+  // on the engine. Will be canceled on engine tear down.
+  std::list<CancelableTaskManager*> task_managers_;
+
   std::unique_ptr<CompilationStatistics> compilation_stats_;
   std::unique_ptr<CodeTracer> code_tracer_;
 

test/cctest/wasm/test-wasm-shared-engine.cc

@@ -117,6 +117,26 @@ class SharedEngineIsolate {
   std::unique_ptr<Zone> zone_;
 };
 
+// Helper class representing a Thread running its own instance of an Isolate
+// with a shared WebAssembly engine available at construction time.
+class SharedEngineThread : public v8::base::Thread {
+ public:
+  SharedEngineThread(SharedEngine* engine,
+                     std::function<void(SharedEngineIsolate&)> callback)
+      : Thread(Options("SharedEngineThread")),
+        engine_(engine),
+        callback_(callback) {}
+
+  virtual void Run() {
+    SharedEngineIsolate isolate(engine_);
+    callback_(isolate);
+  }
+
+ private:
+  SharedEngine* engine_;
+  std::function<void(SharedEngineIsolate&)> callback_;
+};
+
 namespace {
 
 ZoneBuffer* BuildReturnConstantModule(Zone* zone, int constant) {
@@ -192,6 +212,52 @@ TEST(SharedEngineRunImported) {
   CHECK_EQ(1, module.use_count());
 }
 
+TEST(SharedEngineRunThreadedBuilding) {
+  SharedEngine engine;
+  SharedEngineThread thread1(&engine, [](SharedEngineIsolate& isolate) {
+    HandleScope scope(isolate.isolate());
+    ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 23);
+    Handle<WasmInstanceObject> instance = isolate.CompileAndInstantiate(buffer);
+    CHECK_EQ(23, isolate.Run(instance));
+  });
+  SharedEngineThread thread2(&engine, [](SharedEngineIsolate& isolate) {
+    HandleScope scope(isolate.isolate());
+    ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 42);
+    Handle<WasmInstanceObject> instance = isolate.CompileAndInstantiate(buffer);
+    CHECK_EQ(42, isolate.Run(instance));
+  });
+  thread1.Start();
+  thread2.Start();
+  thread1.Join();
+  thread2.Join();
+}
+
+TEST(SharedEngineRunThreadedExecution) {
+  SharedEngine engine;
+  SharedModule module;
+  {
+    SharedEngineIsolate isolate(&engine);
+    HandleScope scope(isolate.isolate());
+    ZoneBuffer* buffer = BuildReturnConstantModule(isolate.zone(), 23);
+    Handle<WasmInstanceObject> instance = isolate.CompileAndInstantiate(buffer);
+    module = isolate.ExportInstance(instance);
+  }
+  SharedEngineThread thread1(&engine, [module](SharedEngineIsolate& isolate) {
+    HandleScope scope(isolate.isolate());
+    Handle<WasmInstanceObject> instance = isolate.ImportInstance(module);
+    CHECK_EQ(23, isolate.Run(instance));
+  });
+  SharedEngineThread thread2(&engine, [module](SharedEngineIsolate& isolate) {
+    HandleScope scope(isolate.isolate());
+    Handle<WasmInstanceObject> instance = isolate.ImportInstance(module);
+    CHECK_EQ(23, isolate.Run(instance));
+  });
+  thread1.Start();
+  thread2.Start();
+  thread1.Join();
+  thread2.Join();
+}
+
 }  // namespace test_wasm_shared_engine
 }  // namespace wasm
 }  // namespace internal