cameron314 created this revision.
cameron314 added reviewers: zturner, clayborg.
Herald added a subscriber: lldb-commits.
This patch changes the order that mutexes are acquired in SBProcess such that
the target API mutex is now always acquired before the public run lock mutex is
acquired.
This fixes a deadlock in LLDB when calling `SBProcess::Kill()` while calling
e.g. `SBProcess::GetThreadByID()`: `Kill` takes the target API mutex, then
waits for the private state thread to exit, which tries to lock the public run
lock mutex, but it's already being held by `GetThreadByID`, which is waiting in
turn for the target API mutex.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D53412
Files:
source/API/SBProcess.cpp
Index: source/API/SBProcess.cpp
===================================================================
--- source/API/SBProcess.cpp
+++ source/API/SBProcess.cpp
@@ -197,11 +197,10 @@
uint32_t num_threads = 0;
ProcessSP process_sp(GetSP());
if (process_sp) {
- Process::StopLocker stop_locker;
-
- const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock());
std::lock_guard<std::recursive_mutex> guard(
process_sp->GetTarget().GetAPIMutex());
+ Process::StopLocker stop_locker;
+ const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock());
num_threads = process_sp->GetThreadList().GetSize(can_update);
}
@@ -457,10 +456,10 @@
ThreadSP thread_sp;
ProcessSP process_sp(GetSP());
if (process_sp) {
- Process::StopLocker stop_locker;
- const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock());
std::lock_guard<std::recursive_mutex> guard(
process_sp->GetTarget().GetAPIMutex());
+ Process::StopLocker stop_locker;
+ const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock());
thread_sp = process_sp->GetThreadList().GetThreadAtIndex(index, can_update);
sb_thread.SetThread(thread_sp);
}
@@ -480,10 +479,10 @@
uint32_t num_queues = 0;
ProcessSP process_sp(GetSP());
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
num_queues = process_sp->GetQueueList().GetSize();
}
}
@@ -502,10 +501,10 @@
QueueSP queue_sp;
ProcessSP process_sp(GetSP());
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
queue_sp = process_sp->GetQueueList().GetQueueAtIndex(index);
sb_queue.SetQueue(queue_sp);
}
@@ -816,10 +815,10 @@
ThreadSP thread_sp;
ProcessSP process_sp(GetSP());
if (process_sp) {
- Process::StopLocker stop_locker;
- const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock());
std::lock_guard<std::recursive_mutex> guard(
process_sp->GetTarget().GetAPIMutex());
+ Process::StopLocker stop_locker;
+ const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock());
thread_sp = process_sp->GetThreadList().FindThreadByID(tid, can_update);
sb_thread.SetThread(thread_sp);
}
@@ -839,10 +838,10 @@
ThreadSP thread_sp;
ProcessSP process_sp(GetSP());
if (process_sp) {
- Process::StopLocker stop_locker;
- const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock());
std::lock_guard<std::recursive_mutex> guard(
process_sp->GetTarget().GetAPIMutex());
+ Process::StopLocker stop_locker;
+ const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock());
thread_sp =
process_sp->GetThreadList().FindThreadByIndexID(index_id, can_update);
sb_thread.SetThread(thread_sp);
@@ -959,10 +958,10 @@
static_cast<void *>(sb_error.get()));
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
bytes_read = process_sp->ReadMemory(addr, dst, dst_len, sb_error.ref());
} else {
if (log)
@@ -993,10 +992,10 @@
size_t bytes_read = 0;
ProcessSP process_sp(GetSP());
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
bytes_read = process_sp->ReadCStringFromMemory(addr, (char *)buf, size,
sb_error.ref());
} else {
@@ -1018,10 +1017,10 @@
uint64_t value = 0;
ProcessSP process_sp(GetSP());
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
value = process_sp->ReadUnsignedIntegerFromMemory(addr, byte_size, 0,
sb_error.ref());
} else {
@@ -1043,10 +1042,10 @@
lldb::addr_t ptr = LLDB_INVALID_ADDRESS;
ProcessSP process_sp(GetSP());
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
ptr = process_sp->ReadPointerFromMemory(addr, sb_error.ref());
} else {
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
@@ -1078,10 +1077,10 @@
static_cast<void *>(sb_error.get()));
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
bytes_written =
process_sp->WriteMemory(addr, src, src_len, sb_error.ref());
} else {
@@ -1158,16 +1157,15 @@
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
ProcessSP process_sp(GetSP());
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
if (log)
log->Printf("SBProcess(%p)::LoadImage() => calling Platform::LoadImage"
"for: %s",
static_cast<void *>(process_sp.get()),
sb_local_image_spec.GetFilename());
-
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
PlatformSP platform_sp = process_sp->GetTarget().GetPlatform();
return platform_sp->LoadImage(process_sp.get(), *sb_local_image_spec,
*sb_remote_image_spec, sb_error.ref());
@@ -1194,16 +1192,15 @@
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
ProcessSP process_sp(GetSP());
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
if (log)
log->Printf("SBProcess(%p)::LoadImageUsingPaths() => "
"calling Platform::LoadImageUsingPaths for: %s",
static_cast<void *>(process_sp.get()),
image_spec.GetFilename());
-
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
PlatformSP platform_sp = process_sp->GetTarget().GetPlatform();
size_t num_paths = paths.GetSize();
std::vector<std::string> paths_vec;
@@ -1242,10 +1239,10 @@
lldb::SBError sb_error;
ProcessSP process_sp(GetSP());
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
PlatformSP platform_sp = process_sp->GetTarget().GetPlatform();
sb_error.SetError(
platform_sp->UnloadImage(process_sp.get(), image_token));
@@ -1265,10 +1262,10 @@
lldb::SBError sb_error;
ProcessSP process_sp(GetSP());
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
sb_error.SetError(process_sp->SendEventData(event_data));
} else {
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
@@ -1364,10 +1361,10 @@
MemoryRegionInfoSP region_info_sp =
std::make_shared<lldb_private::MemoryRegionInfo>();
if (process_sp) {
+ std::lock_guard<std::recursive_mutex> guard(
+ process_sp->GetTarget().GetAPIMutex());
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process_sp->GetRunLock())) {
- std::lock_guard<std::recursive_mutex> guard(
- process_sp->GetTarget().GetAPIMutex());
sb_error.ref() =
process_sp->GetMemoryRegionInfo(load_addr, *region_info_sp);
if (sb_error.Success()) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
