Commit 777d5084 authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

[wasm][gc] Fix deadlock during shutdown

The destructor of the {WasmGCForegroundTask} can be called immediately
when scheduling that task (if the platform determines that the task can
never execute anyway). In that case, we deregister the task from the
wasm engine so we do not access it later (which would be UAF). This
deregistration leads to recursively taking a mutex now.
The only later access to the task happens to cancel the task. For this
purpose, we can also use the {CancelableTaskManager} of the isolate,
and avoid all code in the destructor. This should fix the reentrant
mutex, which leads to a DCHECK failure in debug builds and deadlock
in release builds.

R=mstarzinger@chromium.org

Bug: chromium:984970, v8:8217
Change-Id: I14f05a21ea961ecc391dc59af3b5eebf31e0f873
Cq-Include-Trybots: luci.v8.try:v8_linux_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1706480Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62804}
parent 074fdf1f
......@@ -101,24 +101,12 @@ void CheckNoArchivedThreads(Isolate* isolate) {
isolate->thread_manager()->IterateArchivedThreads(&archived_threads_visitor);
}
class WasmGCForegroundTask : public Task {
class WasmGCForegroundTask : public CancelableTask {
public:
explicit WasmGCForegroundTask(Isolate* isolate) : isolate_(isolate) {
DCHECK_NOT_NULL(isolate);
}
explicit WasmGCForegroundTask(Isolate* isolate)
: CancelableTask(isolate->cancelable_task_manager()), isolate_(isolate) {}
~WasmGCForegroundTask() {
// If the isolate is already shutting down, the platform can delete this
// task without ever executing it. For that case, we need to deregister the
// task from the engine to avoid UAF.
if (isolate_) {
WasmEngine* engine = isolate_->wasm_engine();
engine->ReportLiveCodeForGC(isolate_, Vector<WasmCode*>{});
}
}
void Run() final {
if (isolate_ == nullptr) return; // cancelled.
void RunInternal() final {
WasmEngine* engine = isolate_->wasm_engine();
// If the foreground task is executing, there is no wasm code active. Just
// report an empty set of live wasm code.
......@@ -129,12 +117,8 @@ class WasmGCForegroundTask : public Task {
#endif
CheckNoArchivedThreads(isolate_);
engine->ReportLiveCodeForGC(isolate_, Vector<WasmCode*>{});
// Cancel to signal to the destructor that this task executed.
Cancel();
}
void Cancel() { isolate_ = nullptr; }
private:
Isolate* isolate_;
};
......@@ -913,11 +897,7 @@ void WasmEngine::TriggerGC(int8_t gc_sequence_index) {
bool WasmEngine::RemoveIsolateFromCurrentGC(Isolate* isolate) {
DCHECK(!mutex_.TryLock());
DCHECK_NOT_NULL(current_gc_info_);
auto it = current_gc_info_->outstanding_isolates.find(isolate);
if (it == current_gc_info_->outstanding_isolates.end()) return false;
if (auto* fg_task = it->second) fg_task->Cancel();
current_gc_info_->outstanding_isolates.erase(it);
return true;
return current_gc_info_->outstanding_isolates.erase(isolate) != 0;
}
void WasmEngine::PotentiallyFinishCurrentGC() {
......
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