[wasm] Fix potential deadlock on profiling runs
The deadlock occurs when the Isolate is destroyed before a wasm compile job is finished causing the `WasmEngine::LogCode` to deadlock itself when the TaskRunner is already in the terminated state. Change-Id: I36dc68aecbcb2054d7da61d22defd0eff3130f62 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3976515 Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Matthias Liedtke <mliedtke@chromium.org> Reviewed-by: Andreas Haas <ahaas@chromium.org> Auto-Submit: Matthias Liedtke <mliedtke@chromium.org> Cr-Commit-Position: refs/heads/main@{#83921}
This commit is contained in:
parent
4d95ff1a21
commit
ae4280faa3
@ -1085,37 +1085,55 @@ void WasmEngine::RemoveIsolate(Isolate* isolate) {
|
||||
|
||||
void WasmEngine::LogCode(base::Vector<WasmCode*> code_vec) {
|
||||
if (code_vec.empty()) return;
|
||||
base::MutexGuard guard(&mutex_);
|
||||
NativeModule* native_module = code_vec[0]->native_module();
|
||||
DCHECK_EQ(1, native_modules_.count(native_module));
|
||||
for (Isolate* isolate : native_modules_[native_module]->isolates) {
|
||||
DCHECK_EQ(1, isolates_.count(isolate));
|
||||
IsolateInfo* info = isolates_[isolate].get();
|
||||
if (info->log_codes == false) continue;
|
||||
if (info->log_codes_task == nullptr) {
|
||||
auto new_task = std::make_unique<LogCodesTask>(
|
||||
&mutex_, &info->log_codes_task, isolate, this);
|
||||
info->log_codes_task = new_task.get();
|
||||
info->foreground_task_runner->PostTask(std::move(new_task));
|
||||
}
|
||||
if (info->code_to_log.empty()) {
|
||||
isolate->stack_guard()->RequestLogWasmCode();
|
||||
}
|
||||
for (WasmCode* code : code_vec) {
|
||||
DCHECK_EQ(native_module, code->native_module());
|
||||
code->IncRef();
|
||||
}
|
||||
using TaskToSchedule =
|
||||
std::pair<std::shared_ptr<v8::TaskRunner>, std::unique_ptr<LogCodesTask>>;
|
||||
std::vector<TaskToSchedule> to_schedule;
|
||||
{
|
||||
base::MutexGuard guard(&mutex_);
|
||||
NativeModule* native_module = code_vec[0]->native_module();
|
||||
DCHECK_EQ(1, native_modules_.count(native_module));
|
||||
for (Isolate* isolate : native_modules_[native_module]->isolates) {
|
||||
DCHECK_EQ(1, isolates_.count(isolate));
|
||||
IsolateInfo* info = isolates_[isolate].get();
|
||||
if (info->log_codes == false) continue;
|
||||
if (info->log_codes_task == nullptr) {
|
||||
auto new_task = std::make_unique<LogCodesTask>(
|
||||
&mutex_, &info->log_codes_task, isolate, this);
|
||||
info->log_codes_task = new_task.get();
|
||||
// Store the LogCodeTasks to post them outside the WasmEngine::mutex_.
|
||||
// Posting the task in the mutex can cause the following deadlock (only
|
||||
// in d8): When d8 shuts down, it sets a terminate to the task runner.
|
||||
// When the terminate flag in the taskrunner is set, all newly posted
|
||||
// tasks get destroyed immediately. When the LogCodesTask gets
|
||||
// destroyed, it takes the WasmEngine::mutex_ lock to deregister itself
|
||||
// from the IsolateInfo. Therefore, as the LogCodesTask may get
|
||||
// destroyed immediately when it gets posted, it cannot get posted when
|
||||
// the WasmEngine::mutex_ lock is held.
|
||||
to_schedule.emplace_back(info->foreground_task_runner,
|
||||
std::move(new_task));
|
||||
}
|
||||
if (info->code_to_log.empty()) {
|
||||
isolate->stack_guard()->RequestLogWasmCode();
|
||||
}
|
||||
for (WasmCode* code : code_vec) {
|
||||
DCHECK_EQ(native_module, code->native_module());
|
||||
code->IncRef();
|
||||
}
|
||||
|
||||
auto script_it = info->scripts.find(native_module);
|
||||
// If the script does not yet exist, logging will happen later. If the weak
|
||||
// handle is cleared already, we also don't need to log any more.
|
||||
if (script_it == info->scripts.end()) continue;
|
||||
auto& log_entry = info->code_to_log[script_it->second.script_id()];
|
||||
if (!log_entry.source_url) {
|
||||
log_entry.source_url = script_it->second.source_url();
|
||||
auto script_it = info->scripts.find(native_module);
|
||||
// If the script does not yet exist, logging will happen later. If the
|
||||
// weak handle is cleared already, we also don't need to log any more.
|
||||
if (script_it == info->scripts.end()) continue;
|
||||
auto& log_entry = info->code_to_log[script_it->second.script_id()];
|
||||
if (!log_entry.source_url) {
|
||||
log_entry.source_url = script_it->second.source_url();
|
||||
}
|
||||
log_entry.code.insert(log_entry.code.end(), code_vec.begin(),
|
||||
code_vec.end());
|
||||
}
|
||||
log_entry.code.insert(log_entry.code.end(), code_vec.begin(),
|
||||
code_vec.end());
|
||||
}
|
||||
for (auto& [runner, task] : to_schedule) {
|
||||
runner->PostTask(std::move(task));
|
||||
}
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user