[wasm] Fix data race when deleting the CompilationState

When resetting the {unique_ptr} to the {CompilationState} in the
{NativeModule}, what actually happens is that first the pointer stored
in the {unique_ptr} is reset to {nullptr}, then the destructor is
called.
The destructor of {CompilationState} cancels and waits for background
compile jobs. While doing so, background compile jobs still try to
access the {unique_ptr} in the {NativeModule}.

This CL fixes this race by splitting the shutdown in two steps: First,
cancel and wait the background compile jobs, and only later reset the
pointer.

R=ahaas@chromium.org

Bug: v8:8359
No-Tree-Checks: true
Change-Id: Ifa3bdf3424dfd5a4712d33f8ca85f9382b1766a6
Reviewed-on: https://chromium-review.googlesource.com/c/1296486
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56913}

src/wasm/compilation-environment.h

@@ -71,12 +71,14 @@ struct CompilationEnv {
         lower_simd(lower_simd) {}
 };
 
-// The implementation of {CompilationState} and {CompilationStateDeleter} lives
-// in module-compiler.cc.
+// The implementation of {CompilationState}, {CompilationStateDeleter} and
+// {CancelAndWaitCompilationState} lives in module-compiler.cc.
 struct CompilationStateDeleter {
   void operator()(CompilationState* compilation_state) const;
 };
 
+void CancelAndWaitCompilationState(CompilationState* state);
+
 // Wrapper to create a CompilationState exists in order to avoid having
 // the CompilationState in the header file.
 std::unique_ptr<CompilationState, CompilationStateDeleter> NewCompilationState(

src/wasm/module-compiler.cc

@@ -68,6 +68,10 @@ class CompilationState {
   CompilationState(internal::Isolate*, NativeModule*);
   ~CompilationState();
 
+  // Cancel all background compilation and wait for all tasks to finish. Call
+  // this before destructing this object.
+  void CancelAndWait();
+
   // Set the number of compilations unit expected to be executed. Needs to be
   // set before {AddCompilationUnits} is run, which triggers background
   // compilation.
@@ -2835,6 +2839,10 @@ void CompilationStateDeleter::operator()(
   delete compilation_state;
 }
 
+void CancelAndWaitCompilationState(CompilationState* state) {
+  state->CancelAndWait();
+}
+
 std::unique_ptr<CompilationState, CompilationStateDeleter> NewCompilationState(
     Isolate* isolate, NativeModule* native_module) {
   return std::unique_ptr<CompilationState, CompilationStateDeleter>(
@@ -2859,6 +2867,11 @@ CompilationState::CompilationState(internal::Isolate* isolate,
 }
 
 CompilationState::~CompilationState() {
+  DCHECK(background_task_manager_.canceled());
+  DCHECK(foreground_task_manager_.canceled());
+}
+
+void CompilationState::CancelAndWait() {
   background_task_manager_.CancelAndWait();
   foreground_task_manager_.CancelAndWait();
 }

src/wasm/wasm-code-manager.cc

@@ -796,7 +796,9 @@ void NativeModule::DisableTrapHandler() {
 
 NativeModule::~NativeModule() {
   TRACE_HEAP("Deleting native module: %p\n", reinterpret_cast<void*>(this));
-  compilation_state_.reset();  // Cancels tasks, needs to be done first.
+  // Cancel all background compilation before resetting any field of the
+  // NativeModule or freeing anything.
+  CancelAndWaitCompilationState(compilation_state_.get());
   wasm_code_manager_->FreeNativeModule(this);
 }