[Lldb-commits] Fwd: [PATCH] Add Attach message to let RefreshStateAfterStop handle initial thread list

[Ed Maste]

Hi dmalea,

Right now the FreeBSD and Linux ProcessMonitor classes directly
manipulate the thread list when setting up one initial thread after
launch or attach.

The Linux code has a comment
// FIXME: should we be letting UpdateThreadList handle this?
and I believe we should be; directly manipulating the thread list
appears to me to go against the way lldb is intended to manage
threads.

This is a proof of concept patch that adds a new Attach message and
lets the refresh then handle the thread update.  On FreeBSD with and
without this patch I get the same unit test results.

I'm circulating it for comment for now; a change like this will be a
prerequisite for FreeBSD's thread support.

http://llvm-reviews.chandlerc.com/D1565

Files:
  source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
  source/Plugins/Process/POSIX/ProcessMessage.cpp
  source/Plugins/Process/POSIX/ProcessMessage.h
  source/Plugins/Process/POSIX/ProcessPOSIX.cpp

Index: source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
===================================================================
--- source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -855,7 +855,6 @@
 {
     ProcessMonitor *monitor = args->m_monitor;
     ProcessFreeBSD &process = monitor->GetProcess();
-    lldb::ProcessSP processSP = process.shared_from_this();
     const char **argv = args->m_argv;
     const char **envp = args->m_envp;
     const char *stdin_path = args->m_stdin_path;
@@ -868,9 +867,6 @@
     char err_str[err_len];
     lldb::pid_t pid;

-    lldb::ThreadSP inferior;
-    Log *log (ProcessPOSIXLog::GetLogIfAllCategoriesSet (POSIX_LOG_PROCESS));
-
     // Propagate the environment if one is not supplied.
     if (envp == NULL || envp[0] == NULL)
         envp = const_cast<const char **>(environ);
@@ -1002,14 +998,7 @@
     if (!EnsureFDFlags(monitor->m_terminal_fd, O_NONBLOCK, args->m_error))
         goto FINISH;

-    // Update the process thread list with this new thread.
-    inferior.reset(process.CreateNewPOSIXThread(*processSP, pid));
-    if (log)
-        log->Printf ("ProcessMonitor::%s() adding pid = %" PRIu64,
__FUNCTION__, pid);
-    process.GetThreadList().AddThread(inferior);
-
-    // Let our process instance know the thread has stopped.
-    process.SendMessage(ProcessMessage::Trace(pid));
+    process.SendMessage(ProcessMessage::Attach(pid));

 FINISH:
     return args->m_error.Success();
@@ -1059,9 +1048,6 @@

     ProcessMonitor *monitor = args->m_monitor;
     ProcessFreeBSD &process = monitor->GetProcess();
-    lldb::ProcessSP processSP = process.shared_from_this();
-    ThreadList &tl = process.GetThreadList();
-    lldb::ThreadSP inferior;

     if (pid <= 1)
     {
@@ -1084,14 +1070,9 @@
         goto FINISH;
     }

-    // Update the process thread list with the attached thread.
-    inferior.reset(process.CreateNewPOSIXThread(*processSP, pid));
-    tl.AddThread(inferior);
-
-    // Let our process instance know the thread has stopped.
-    process.SendMessage(ProcessMessage::Trace(pid));
+    process.SendMessage(ProcessMessage::Attach(pid));

- FINISH:
+FINISH:
     return args->m_error.Success();
 }

Index: source/Plugins/Process/POSIX/ProcessMessage.cpp
===================================================================
--- source/Plugins/Process/POSIX/ProcessMessage.cpp
+++ source/Plugins/Process/POSIX/ProcessMessage.cpp
@@ -218,6 +218,9 @@
     case eInvalidMessage:
         str = "eInvalidMessage";
         break;
+    case eAttachMessage:
+        str = "eAttachMessage";
+        break;
     case eExitMessage:
         str = "eExitMessage";
         break;
Index: source/Plugins/Process/POSIX/ProcessMessage.h
===================================================================
--- source/Plugins/Process/POSIX/ProcessMessage.h
+++ source/Plugins/Process/POSIX/ProcessMessage.h
@@ -23,6 +23,7 @@
     enum Kind
     {
         eInvalidMessage,
+        eAttachMessage,
         eExitMessage,
         eLimboMessage,
         eSignalMessage,
@@ -79,6 +80,11 @@

     lldb::tid_t GetTID() const { return m_tid; }

+    /// Indicates that the process @p pid has successfully attached.
+    static ProcessMessage Attach(lldb::pid_t pid) {
+        return ProcessMessage(pid, eAttachMessage);
+    }
+
     /// Indicates that the thread @p tid is about to exit with status
@p status.
     static ProcessMessage Limbo(lldb::tid_t tid, int status) {
         return ProcessMessage(tid, eLimboMessage, status);
Index: source/Plugins/Process/POSIX/ProcessPOSIX.cpp
===================================================================
--- source/Plugins/Process/POSIX/ProcessPOSIX.cpp
+++ source/Plugins/Process/POSIX/ProcessPOSIX.cpp
@@ -367,6 +367,10 @@
     case ProcessMessage::eInvalidMessage:
         return;

+    case ProcessMessage::eAttachMessage:
+        SetPrivateState(eStateStopped);
+        return;
+
     case ProcessMessage::eLimboMessage:
         assert(thread);
         thread->SetState(eStateStopped);