[Lldb-commits] [lldb] [lldb-dap] Ensure we acquire the SB API lock while handling requests. (PR #137026)

Tue May 6 15:25:03 PDT 2025

jeffreytan81 wrote:

@ashgti, this change is causing deadlock in our lldb. We have internal changes that main thread's disconnect request handler tried to reset debugger states and call StopEventHandlers() to ensure all event threads to stop. However, this deadlocks now because event thread can call `SBProcess::GetExitStatus()` to acquire the API mutex which will wait forever: 

Main Thread
```
__futex_abstimed_wait_common64 (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/nptl/futex-internal.c:57)
__futex_abstimed_wait_common (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/nptl/futex-internal.c:87)
__GI___futex_abstimed_wait_cancelable64 (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/nptl/futex-internal.c:139)
__pthread_clockjoin_ex (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/nptl/pthread_join_common.c:105)
__gthread_join (/home/engshare/third-party2/libgcc/11.x/src/gcc-11.x/x86_64-facebook-linux/libstdc++-v3/include/x86_64-facebook-linux/bits/gthr-default.h:669)
std::thread::join() (/home/engshare/third-party2/libgcc/11.x/src/gcc-11.x/libstdc++-v3/src/c++11/thread.cc:112)
lldb_dap::DAP::StopEventHandlers() (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/tools/lldb-dap/DAP.cpp:234)
lldb_dap::DAP::ResetDebuggerState() (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/tools/lldb-dap/DAP.cpp:1411)
lldb_dap::DAP::Disconnect(bool) (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/tools/lldb-dap/DAP.cpp:842)
lldb_dap::DisconnectRequestHandler::Run(std::optional<lldb_dap::protocol::DisconnectArguments> const&) const (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/tools/lldb-dap/Handler/DisconnectRequestHandler.cpp:32)
lldb_dap::RequestHandler<std::optional<lldb_dap::protocol::DisconnectArguments>, llvm::Error>::operator()(lldb_dap::protocol::Request const&) const (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/tools/lldb-dap/Handler/RequestHandler.h:140)
lldb_dap::BaseRequestHandler::Run(lldb_dap::protocol::Request const&) (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/tools/lldb-dap/Handler/RequestHandler.cpp:197)
lldb_dap::DAP::HandleObject(std::variant<lldb_dap::protocol::Request, lldb_dap::protocol::Response, lldb_dap::protocol::Event> const&) (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/tools/lldb-dap/DAP.cpp:734)
lldb_dap::DAP::Loop() (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/tools/lldb-dap/DAP.cpp:979)
main (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/tools/lldb-dap/lldb-dap.cpp:620)
__libc_start_call_main (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/sysdeps/nptl/libc_start_call_main.h:58)
__libc_start_main_impl (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/libc-start.c:409)
_start (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/sysdeps/x86_64/start.S:116)
```

Event thread:
```
futex_wait (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/sysdeps/nptl/futex-internal.h:146)
__GI___lll_lock_wait (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/nptl/lowlevellock.c:50)
lll_mutex_lock_optimized (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/nptl/pthread_mutex_lock.c:49)
___pthread_mutex_lock (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/nptl/pthread_mutex_lock.c:124)
__gthread_mutex_lock(pthread_mutex_t*) (/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/x86_64-facebook-linux/bits/gthr-default.h:749)
__gthread_recursive_mutex_lock(pthread_mutex_t*) (/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/x86_64-facebook-linux/bits/gthr-default.h:811)
std::recursive_mutex::lock() (/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/mutex:108)
std::lock_guard<std::recursive_mutex>::lock_guard(std::recursive_mutex&) (/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/bits/std_mutex.h:229)
lldb::SBProcess::GetExitStatus() (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/source/API/SBProcess.cpp:505)
lldb_dap::SendProcessExitedEvent(lldb_dap::DAP&, lldb::SBProcess&) (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/tools/lldb-dap/EventHelper.cpp:269)
lldb_dap::EventThreadFunction(lldb_dap::DAP&) (/data/users/jeffreytan/llvm-sand/external/llvm-project/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp:198)
void std::__invoke_impl<void, void (*)(lldb_dap::DAP&), std::reference_wrapper<lldb_dap::DAP>>(std::__invoke_other, void (*&&)(lldb_dap::DAP&), std::reference_wrapper<lldb_dap::DAP>&&) (/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/bits/invoke.h:61)
std::__invoke_result<void (*)(lldb_dap::DAP&), std::reference_wrapper<lldb_dap::DAP>>::type std::__invoke<void (*)(lldb_dap::DAP&), std::reference_wrapper<lldb_dap::DAP>>(void (*&&)(lldb_dap::DAP&), std::reference_wrapper<lldb_dap::DAP>&&) (/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/bits/invoke.h:96)
void std::thread::_Invoker<std::tuple<void (*)(lldb_dap::DAP&), std::reference_wrapper<lldb_dap::DAP>>>::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) (/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/bits/std_thread.h:253)
std::thread::_Invoker<std::tuple<void (*)(lldb_dap::DAP&), std::reference_wrapper<lldb_dap::DAP>>>::operator()() (/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/bits/std_thread.h:260)
std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(lldb_dap::DAP&), std::reference_wrapper<lldb_dap::DAP>>>>::_M_run() (/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/bits/std_thread.h:211)
std::execute_native_thread_routine(void *) (/home/engshare/third-party2/libgcc/11.x/src/gcc-11.x/libstdc++-v3/src/c++11/thread.cc:82)
start_thread (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/nptl/pthread_create.c:434)
__clone3 (/home/engshare/third-party2/glibc/2.34/src/glibc-2.34/sysdeps/unix/sysv/linux/x86_64/clone3.S:81)
```

High level, I do not think it is a good idea to hold the top level API mutex which is way too large locking scope. We should leave the decision to each request handler for smaller scope locks if they want to ensure a critical section. Can we revert the PR? 

cc @clayborg 

https://github.com/llvm/llvm-project/pull/137026