Commit 383c4a44 authored by Thibaud Michaud's avatar Thibaud Michaud Committed by Commit Bot

[wasm][debug] Fix DebugInfo deadlock

Add a separate mutex for the {debug_side_tables_} field. This ensures
that we can use {GetDebugSideTableIfExists} even if {mutex_} is already
locked.

R=ahaas@chromium.org
CC=​​clemensb@chromium.org

Bug: v8:10889
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
Change-Id: Icb67c45aec0cf66814705b83532f4833f36738e1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2402879
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69888}
parent 241c8fa4
...@@ -450,8 +450,11 @@ class DebugInfoImpl { ...@@ -450,8 +450,11 @@ class DebugInfoImpl {
native_module_->AddCompiledCode(std::move(result))); native_module_->AddCompiledCode(std::move(result)));
DCHECK(new_code->is_inspectable()); DCHECK(new_code->is_inspectable());
DCHECK_EQ(0, debug_side_tables_.count(new_code)); {
debug_side_tables_.emplace(new_code, std::move(debug_sidetable)); base::MutexGuard lock(&debug_side_tables_mutex_);
DCHECK_EQ(0, debug_side_tables_.count(new_code));
debug_side_tables_.emplace(new_code, std::move(debug_sidetable));
}
return new_code; return new_code;
} }
...@@ -610,14 +613,14 @@ class DebugInfoImpl { ...@@ -610,14 +613,14 @@ class DebugInfoImpl {
} }
void RemoveDebugSideTables(Vector<WasmCode* const> codes) { void RemoveDebugSideTables(Vector<WasmCode* const> codes) {
base::MutexGuard guard(&mutex_); base::MutexGuard guard(&debug_side_tables_mutex_);
for (auto* code : codes) { for (auto* code : codes) {
debug_side_tables_.erase(code); debug_side_tables_.erase(code);
} }
} }
DebugSideTable* GetDebugSideTableIfExists(const WasmCode* code) const { DebugSideTable* GetDebugSideTableIfExists(const WasmCode* code) const {
base::MutexGuard guard(&mutex_); base::MutexGuard guard(&debug_side_tables_mutex_);
auto it = debug_side_tables_.find(code); auto it = debug_side_tables_.find(code);
return it == debug_side_tables_.end() ? nullptr : it->second.get(); return it == debug_side_tables_.end() ? nullptr : it->second.get();
} }
...@@ -687,7 +690,7 @@ class DebugInfoImpl { ...@@ -687,7 +690,7 @@ class DebugInfoImpl {
{ {
// Only hold the mutex temporarily. We can't hold it while generating the // Only hold the mutex temporarily. We can't hold it while generating the
// debug side table, because compilation takes the {NativeModule} lock. // debug side table, because compilation takes the {NativeModule} lock.
base::MutexGuard guard(&mutex_); base::MutexGuard guard(&debug_side_tables_mutex_);
auto it = debug_side_tables_.find(code); auto it = debug_side_tables_.find(code);
if (it != debug_side_tables_.end()) return it->second.get(); if (it != debug_side_tables_.end()) return it->second.get();
} }
...@@ -708,7 +711,7 @@ class DebugInfoImpl { ...@@ -708,7 +711,7 @@ class DebugInfoImpl {
// Check cache again, maybe another thread concurrently generated a debug // Check cache again, maybe another thread concurrently generated a debug
// side table already. // side table already.
{ {
base::MutexGuard guard(&mutex_); base::MutexGuard guard(&debug_side_tables_mutex_);
auto& slot = debug_side_tables_[code]; auto& slot = debug_side_tables_[code];
if (slot != nullptr) return slot.get(); if (slot != nullptr) return slot.get();
slot = std::move(debug_side_table); slot = std::move(debug_side_table);
...@@ -847,13 +850,15 @@ class DebugInfoImpl { ...@@ -847,13 +850,15 @@ class DebugInfoImpl {
NativeModule* const native_module_; NativeModule* const native_module_;
// {mutex_} protects all fields below. mutable base::Mutex debug_side_tables_mutex_;
mutable base::Mutex mutex_;
// DebugSideTable per code object, lazily initialized. // DebugSideTable per code object, lazily initialized.
std::unordered_map<const WasmCode*, std::unique_ptr<DebugSideTable>> std::unordered_map<const WasmCode*, std::unique_ptr<DebugSideTable>>
debug_side_tables_; debug_side_tables_;
// {mutex_} protects all fields below.
mutable base::Mutex mutex_;
// Names of locals, lazily decoded from the wire bytes. // Names of locals, lazily decoded from the wire bytes.
std::unique_ptr<LocalNames> local_names_; std::unique_ptr<LocalNames> local_names_;
......
...@@ -574,6 +574,13 @@ WASM_COMPILED_EXEC_TEST(WasmBreakInPostMVP) { ...@@ -574,6 +574,13 @@ WASM_COMPILED_EXEC_TEST(WasmBreakInPostMVP) {
CHECK_EQ(kReturn, GetIntReturnValue(retval)); CHECK_EQ(kReturn, GetIntReturnValue(retval));
} }
WASM_COMPILED_EXEC_TEST(Regress10889) {
FLAG_SCOPE(print_wasm_code);
WasmRunner<int> runner(execution_tier);
BUILD(runner, WASM_I32V_1(0));
SetBreakpoint(&runner, runner.function_index(), 1, 1);
}
} // namespace wasm } // namespace wasm
} // namespace internal } // namespace internal
} // namespace v8 } // namespace v8
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