Commit 62d3ea84 authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

[wasm] Avoid accidentally creating two foreground tasks

If we create a second foreground task, only the second one will be
registered with the AsyncCompileJob, so the first one will not be
cancelled, which can lead to use-after-free of the AsyncCompileJob.
In a debug build, a DCHECK will fail when creating the second
foreground task.

R=ahaas@chromium.org

Bug: chromium:907937, chromium:910920
Change-Id: Iefefc4a85e7b35b32051cfe8cd5cbbfc4e95b843
Reviewed-on: https://chromium-review.googlesource.com/c/1367684
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58108}
parent e2ebe350
......@@ -2463,7 +2463,7 @@ class AsyncCompileJob::CompilationStateCallback {
error = handle(*error, job_->isolate());
job_->deferred_handles_.push_back(deferred.Detach());
job_->DoSync<CompileFailed>(error);
job_->DoSync<CompileFailed, kUseExistingForegroundTask>(error);
}
break;
......@@ -2578,9 +2578,12 @@ void AsyncCompileJob::StartBackgroundTask() {
}
}
template <typename Step, typename... Args>
template <typename Step,
AsyncCompileJob::UseExistingForegroundTask use_existing_fg_task,
typename... Args>
void AsyncCompileJob::DoSync(Args&&... args) {
NextStep<Step>(std::forward<Args>(args)...);
if (use_existing_fg_task && pending_foreground_task_ != nullptr) return;
StartForegroundTask();
}
......@@ -2780,11 +2783,9 @@ void AsyncStreamingProcessor::FinishAsyncCompileJobWithError(ResultBase error) {
if (job_->native_module_) {
Impl(job_->native_module_->compilation_state())->Abort();
if (job_->pending_foreground_task_ == nullptr) {
job_->DoSync<AsyncCompileJob::DecodeFail>(std::move(result));
} else {
job_->NextStep<AsyncCompileJob::DecodeFail>(std::move(result));
}
job_->DoSync<AsyncCompileJob::DecodeFail,
AsyncCompileJob::kUseExistingForegroundTask>(
std::move(result));
// Clear the {compilation_unit_builder_} if it exists. This is needed
// because there is a check in the destructor of the
......
......@@ -118,9 +118,17 @@ class AsyncCompileJob {
void StartBackgroundTask();
enum UseExistingForegroundTask : bool {
kUseExistingForegroundTask = true,
kAssertNoExistingForegroundTask = false
};
// Switches to the compilation step {Step} and starts a foreground task to
// execute it.
template <typename Step, typename... Args>
// execute it. Most of the time we know that there cannot be a running
// foreground task. If there might be one, then pass
// kUseExistingForegroundTask to avoid spawning a second one.
template <typename Step,
UseExistingForegroundTask = kAssertNoExistingForegroundTask,
typename... Args>
void DoSync(Args&&... args);
// Switches to the compilation step {Step} and immediately executes that step.
......
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