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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to