[wasm] Notify StreamingDecoder when its AsyncCompileJob is destructed

gahaas · Commit Bot · commit 1a5a6c3619fa · 2018-09-25T14:30:54.000Z
The lifetime of the AsyncCompileJob does not depend on the lifetime of the stream which feeds data into it. Multiple checks guarantee that the AsyncCompileJob still exists when the stream wants to call it. With this CL we add an additional level of defense to make sure that streaming does not continue after the AsyncCompileJob got destructed. It is not clear if this CL fixes the bug referenced below. However, the crashes there could be caused when streaming accesses the AsyncCompileJob after it got destructed already. I was not able though to find a scenario where this is possible. R=clemensh@chromium.org Bug: chromium:888170 Change-Id: Id5c6cc34842735a3adaf3e09c57cbe923cfc2630 Reviewed-on: https://chromium-review.googlesource.com/1241961 Commit-Queue: Andreas Haas <ahaas@chromium.org> Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#56213}
diff --git a/src/wasm/module-compiler.cc b/src/wasm/module-compiler.cc
@@ -2254,6 +2254,11 @@ std::shared_ptr<StreamingDecoder> AsyncCompileJob::CreateStreamingDecoder() {
 AsyncCompileJob::~AsyncCompileJob() {
   background_task_manager_.CancelAndWait();
   if (native_module_) native_module_->compilation_state()->Abort();
+  // Tell the streaming decoder that the AsyncCompileJob is not available
+  // anymore.
+  // TODO(ahaas): Is this notification really necessary? Check
+  // https://crbug.com/888170.
+  if (stream_) stream_->NotifyCompilationEnded();
   CancelPendingForegroundTask();
   for (auto d : deferred_handles_) delete d;
 }
@@ -2285,7 +2290,6 @@ void AsyncCompileJob::FinishCompile() {
 }
 
 void AsyncCompileJob::AsyncCompileFailed(Handle<Object> error_reason) {
-  if (stream_) stream_->NotifyError();
   // {job} keeps the {this} pointer alive.
   std::shared_ptr<AsyncCompileJob> job =
       isolate_->wasm_engine()->RemoveCompileJob(this);
diff --git a/src/wasm/streaming-decoder.h b/src/wasm/streaming-decoder.h
@@ -65,8 +65,13 @@ class V8_EXPORT_PRIVATE StreamingDecoder {
 
   void Abort();
 
-  // Notify the StreamingDecoder that there has been an compilation error.
-  void NotifyError() { ok_ = false; }
+  // Notify the StreamingDecoder that compilation ended and the
+  // StreamingProcessor should not be called anymore.
+  void NotifyCompilationEnded() {
+    // We set {ok_} to false to turn all future calls to the StreamingDecoder
+    // into no-ops.
+    ok_ = false;
+  }
 
  private:
   // TODO(ahaas): Put the whole private state of the StreamingDecoder into the
