labath created this revision.
labath added reviewers: mgorny, DavidSpickett.
Herald added a subscriber: krytarowski.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.
This patch handles the situation where the main thread exits (through
the SYS_exit syscall). In this case, the process as a whole continues
running until all of the other threads exit, or one of them issues an
exit_group syscall.
The patch consists of two changes:
- a moderate redesign of the handling of thread exit (WIFEXITED) events.
Previously, we were removing (forgetting) a thread once we received the
WIFEXITED (or WIFSIGNALED) event. This was problematic for the main thread,
since the main thread WIFEXITED event (which is better thought of as a
process-wide event) gets reported only after the entire process exits. This
resulted in deadlocks, where we were waiting for the process to stop (because
we still considered the main thread "live").
This patch changes the logic such that the main thread is removed as soon as
its PTRACE_EVENT_EXIT (the pre-exit) event is received. At this point we can
consider the thread gone (for most purposes). As a corrolary, I needed to add
special logic to catch process-wide exit events in the cases where we don't
have the main thread around.
- The second part of the patch is the removal of the assumptions that the main
thread is always available. This generally meant replacing the uses of
GetThreadByID(process_id) with GetCurrentThread() in various process-wide
operations (such as memory reads).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D122716
Files:
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/test/API/functionalities/thread/main_thread_exit/Makefile
lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py
lldb/test/API/functionalities/thread/main_thread_exit/main.cpp
Index: lldb/test/API/functionalities/thread/main_thread_exit/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/thread/main_thread_exit/main.cpp
@@ -0,0 +1,23 @@
+#include <thread>
+
+#ifdef __linux__
+#include <sys/syscall.h>
+#include <unistd.h>
+
+void exit_thread(int result) { syscall(SYS_exit, result); }
+#else
+#error Needs OS-specific implementation
+#endif
+
+int call_me() { return 12345; }
+
+void thread() {
+ std::this_thread::sleep_for(
+ std::chrono::seconds(10)); // Let the main thread exit.
+ exit_thread(42); // break here
+}
+
+int main() {
+ std::thread(thread).detach();
+ exit_thread(47);
+}
Index: lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py
@@ -0,0 +1,31 @@
+"""
+Test handling of the situation where the main thread exits but the other threads
+in the process keep running.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class ThreadExitTestCase(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+ NO_DEBUG_INFO_TESTCASE = True
+
+ # Needs os-specific implementation in the inferior
+ @skipIf(oslist=no_match(["linux"]))
+ def test(self):
+ self.build()
+ lldbutil.run_to_source_breakpoint(self, "// break here",
+ lldb.SBFileSpec("main.cpp"))
+
+ # There should be one (non-main) thread left
+ self.assertEquals(self.process().GetNumThreads(), 1)
+
+ # Ensure we can evaluate_expressions in this state
+ self.expect_expr("call_me()", result_value="12345")
+
+ self.runCmd("continue")
+ self.assertEquals(self.process().GetExitStatus(), 47)
Index: lldb/test/API/functionalities/thread/main_thread_exit/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/thread/main_thread_exit/Makefile
@@ -0,0 +1,3 @@
+ENABLE_THREADS := YES
+CXX_SOURCES := main.cpp
+include Makefile.rules
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -447,13 +447,7 @@
// This is a thread that exited. Ensure we're not tracking it anymore.
StopTrackingThread(thread);
- if (is_main_thread) {
- // The main thread exited. We're done monitoring. Report to delegate.
- SetExitStatus(status, true);
-
- // Notify delegate that our process has exited.
- SetState(StateType::eStateExited, true);
- }
+ assert(!is_main_thread && "Main thread exits handled elsewhere");
return;
}
@@ -611,6 +605,13 @@
}
ResumeThread(thread, state, LLDB_INVALID_SIGNAL_NUMBER);
+ if (is_main_thread) {
+ // Main thread report the read (WIFEXITED) event only after all threads in
+ // the process exit, so we need to stop tracking it here instead of in
+ // MonitorCallback
+ StopTrackingThread(thread);
+ }
+
break;
}
@@ -1177,11 +1178,11 @@
// Linux kernel since 2.6.14 has /proc/{pid}/smaps
// if CONFIG_PROC_PAGE_MONITOR is enabled
- auto BufferOrError = getProcFile(GetID(), "smaps");
+ auto BufferOrError = getProcFile(GetID(), GetCurrentThreadID(), "smaps");
if (BufferOrError)
ParseLinuxSMapRegions(BufferOrError.get()->getBuffer(), callback);
else {
- BufferOrError = getProcFile(GetID(), "maps");
+ BufferOrError = getProcFile(GetID(), GetCurrentThreadID(), "maps");
if (!BufferOrError) {
m_supports_mem_region = LazyBool::eLazyBoolNo;
return BufferOrError.getError();
@@ -1232,7 +1233,7 @@
addr_t exe_addr = region_it->first.GetRange().GetRangeBase();
- NativeThreadLinux &thread = *GetThreadByID(GetID());
+ NativeThreadLinux &thread = *GetCurrentThread();
assert(thread.GetState() == eStateStopped);
NativeRegisterContextLinux ®_ctx = thread.GetRegisterContext();
@@ -1384,8 +1385,9 @@
tags_iovec.iov_len = num_tags;
Status error = NativeProcessLinux::PtraceWrapper(
- details->ptrace_read_req, GetID(), reinterpret_cast<void *>(read_addr),
- static_cast<void *>(&tags_iovec), 0, nullptr);
+ details->ptrace_read_req, GetCurrentThreadID(),
+ reinterpret_cast<void *>(read_addr), static_cast<void *>(&tags_iovec),
+ 0, nullptr);
if (error.Fail()) {
// Discard partial reads
@@ -1453,7 +1455,7 @@
tags_vec.iov_len = num_tags;
Status error = NativeProcessLinux::PtraceWrapper(
- details->ptrace_write_req, GetID(),
+ details->ptrace_write_req, GetCurrentThreadID(),
reinterpret_cast<void *>(write_addr), static_cast<void *>(&tags_vec), 0,
nullptr);
@@ -1525,15 +1527,14 @@
// The process_vm_readv path is about 50 times faster than ptrace api. We
// want to use this syscall if it is supported.
- const ::pid_t pid = GetID();
-
struct iovec local_iov, remote_iov;
local_iov.iov_base = buf;
local_iov.iov_len = size;
remote_iov.iov_base = reinterpret_cast<void *>(addr);
remote_iov.iov_len = size;
- bytes_read = process_vm_readv(pid, &local_iov, 1, &remote_iov, 1, 0);
+ bytes_read = process_vm_readv(GetCurrentThreadID(), &local_iov, 1,
+ &remote_iov, 1, 0);
const bool success = bytes_read == size;
Log *log = GetLog(POSIXLog::Process);
@@ -1557,7 +1558,7 @@
for (bytes_read = 0; bytes_read < size; bytes_read += remainder) {
Status error = NativeProcessLinux::PtraceWrapper(
- PTRACE_PEEKDATA, GetID(), (void *)addr, nullptr, 0, &data);
+ PTRACE_PEEKDATA, GetCurrentThreadID(), (void *)addr, nullptr, 0, &data);
if (error.Fail())
return error;
@@ -1592,8 +1593,8 @@
memcpy(&data, src, k_ptrace_word_size);
LLDB_LOG(log, "[{0:x}]:{1:x}", addr, data);
- error = NativeProcessLinux::PtraceWrapper(PTRACE_POKEDATA, GetID(),
- (void *)addr, (void *)data);
+ error = NativeProcessLinux::PtraceWrapper(
+ PTRACE_POKEDATA, GetCurrentThreadID(), (void *)addr, (void *)data);
if (error.Fail())
return error;
} else {
@@ -1857,46 +1858,64 @@
}
}
+static llvm::Optional<WaitStatus> HandlePid(::pid_t pid) {
+ Log *log = GetLog(POSIXLog::Process);
+
+ int status;
+ ::pid_t wait_pid = llvm::sys::RetryAfterSignal(
+ -1, ::waitpid, pid, &status, __WALL | __WNOTHREAD | WNOHANG);
+
+ if (wait_pid == 0)
+ return llvm::None;
+
+ if (wait_pid == -1) {
+ Status error(errno, eErrorTypePOSIX);
+ LLDB_LOG(log, "waitpid({0}, &status, _) failed: {1}", pid,
+ error);
+ return llvm::None;
+ }
+
+ assert(wait_pid == pid);
+
+ WaitStatus wait_status = WaitStatus::Decode(status);
+
+ LLDB_LOG(log, "waitpid({0}) got status = {1}", pid, wait_status);
+ return wait_status;
+}
+
void NativeProcessLinux::SigchldHandler() {
Log *log = GetLog(POSIXLog::Process);
// Threads can appear or disappear as a result of event processing, so gather
// the events upfront.
llvm::DenseMap<lldb::tid_t, WaitStatus> tid_events;
+ bool checked_main_thread = false;
for (const auto &thread_up : m_threads) {
- int status = -1;
- ::pid_t wait_pid =
- llvm::sys::RetryAfterSignal(-1, ::waitpid, thread_up->GetID(), &status,
- __WALL | __WNOTHREAD | WNOHANG);
-
- if (wait_pid == 0)
- continue; // Nothing to do for this thread.
-
- if (wait_pid == -1) {
- Status error(errno, eErrorTypePOSIX);
- LLDB_LOG(log, "waitpid({0}, &status, _) failed: {1}", thread_up->GetID(),
- error);
- continue;
- }
+ if (thread_up->GetID() == GetID())
+ checked_main_thread = true;
- assert(wait_pid == static_cast<::pid_t>(thread_up->GetID()));
-
- WaitStatus wait_status = WaitStatus::Decode(status);
-
- LLDB_LOG(log, "waitpid({0}) got status = {1}", thread_up->GetID(),
- wait_status);
- tid_events.try_emplace(thread_up->GetID(), wait_status);
+ if (llvm::Optional<WaitStatus> status = HandlePid(thread_up->GetID()))
+ tid_events.try_emplace(thread_up->GetID(), *status);
+ }
+ // Check the main thread even when we're not tracking it as process exit
+ // events are reported that way.
+ if (!checked_main_thread) {
+ if (llvm::Optional<WaitStatus> status = HandlePid(GetID()))
+ tid_events.try_emplace(GetID(), *status);
}
for (auto &KV : tid_events) {
LLDB_LOG(log, "processing {0}({1}) ...", KV.first, KV.second);
- NativeThreadLinux *thread = GetThreadByID(KV.first);
- if (thread) {
- MonitorCallback(*thread, KV.second);
- } else {
- // This can happen if one of the events is an main thread exit.
- LLDB_LOG(log, "... but the thread has disappeared");
+ if (KV.first == GetID() && (KV.second.type == WaitStatus::Exit ||
+ KV.second.type == WaitStatus::Signal)) {
+
+ // The process exited. We're done monitoring. Report to delegate.
+ SetExitStatus(KV.second, true);
+ return;
}
+ NativeThreadLinux *thread = GetThreadByID(KV.first);
+ assert(thread && "Why did this thread disappear?");
+ MonitorCallback(*thread, KV.second);
}
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
