Remove shared pointer from NativeProcessProtocol

Summary:
The usage of shared_from_this forces us to separate construction and
initialization phases, because shared_from_this() is not available in
the constructor (or destructor). The shared semantics are not necessary,
as we always have a clear owner of the native process class
(GDBRemoteCommunicationServerLLDB object). Even if we need shared
semantics in the future (which I think we should strongly avoid),
reverting this will not be necessary -- the owners can still easily
store the native process object in a shared pointer if they really want
to -- this just prevents the knowledge of that from leaking into the
class implementation.

After this a NativeThread object will hold a reference to the parent
process (instead of a weak_ptr) -- having a process instance always
available allows us to simplify some logic in this class (some of it was
already simplified because we were asserting that the process is
available, but this makes it obvious).

Reviewers: krytarowski, eugene, zturner

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D35123

llvm-svn: 308282

5 files changed, 21 insertions, 44 deletions

diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index d988ee93a2b..170d3b10006 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -214,7 +214,7 @@ static Status EnsureFDFlags(int fd, int flags) {
 // Public Static Methods
 // -----------------------------------------------------------------------------
 
-llvm::Expected<NativeProcessProtocolSP>
+llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
 NativeProcessLinux::Factory::Launch(ProcessLaunchInfo &launch_info,
                                     NativeDelegate &native_delegate,
                                     MainLoop &mainloop) const {
@@ -259,14 +259,13 @@ NativeProcessLinux::Factory::Launch(ProcessLaunchInfo &launch_info,
     return status.ToError();
   }
 
-  std::shared_ptr<NativeProcessLinux> process_sp(new NativeProcessLinux(
+  return std::unique_ptr<NativeProcessLinux>(new NativeProcessLinux(
       pid, launch_info.GetPTY().ReleaseMasterFileDescriptor(), native_delegate,
-      arch, mainloop));
-  process_sp->InitializeThreads({pid});
-  return process_sp;
+      arch, mainloop, {pid}));
 }
 
-llvm::Expected<NativeProcessProtocolSP> NativeProcessLinux::Factory::Attach(
+llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
+NativeProcessLinux::Factory::Attach(
     lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate,
     MainLoop &mainloop) const {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
@@ -282,10 +281,8 @@ llvm::Expected<NativeProcessProtocolSP> NativeProcessLinux::Factory::Attach(
   if (!tids_or)
     return tids_or.takeError();
 
-  std::shared_ptr<NativeProcessLinux> process_sp(
-      new NativeProcessLinux(pid, -1, native_delegate, arch, mainloop));
-  process_sp->InitializeThreads(*tids_or);
-  return process_sp;
+  return std::unique_ptr<NativeProcessLinux>(new NativeProcessLinux(
+      pid, -1, native_delegate, arch, mainloop, *tids_or));
 }
 
 // -----------------------------------------------------------------------------
@@ -294,7 +291,8 @@ llvm::Expected<NativeProcessProtocolSP> NativeProcessLinux::Factory::Attach(
 
 NativeProcessLinux::NativeProcessLinux(::pid_t pid, int terminal_fd,
                                        NativeDelegate &delegate,
-                                       const ArchSpec &arch, MainLoop &mainloop)
+                                       const ArchSpec &arch, MainLoop &mainloop,
+                                       llvm::ArrayRef<::pid_t> tids)
     : NativeProcessProtocol(pid, terminal_fd, delegate), m_arch(arch) {
   if (m_terminal_fd != -1) {
     Status status = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
@@ -305,9 +303,7 @@ NativeProcessLinux::NativeProcessLinux(::pid_t pid, int terminal_fd,
   m_sigchld_handle = mainloop.RegisterSignal(
       SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, status);
   assert(m_sigchld_handle && status.Success());
-}
 
-void NativeProcessLinux::InitializeThreads(llvm::ArrayRef<::pid_t> tids) {
   for (const auto &tid : tids) {
     NativeThreadLinuxSP thread_sp = AddThread(tid);
     assert(thread_sp && "AddThread() returned a nullptr thread");
@@ -2009,7 +2005,7 @@ NativeThreadLinuxSP NativeProcessLinux::AddThread(lldb::tid_t thread_id) {
   if (m_threads.empty())
     SetCurrentThreadID(thread_id);
 
-  auto thread_sp = std::make_shared<NativeThreadLinux>(this, thread_id);
+  auto thread_sp = std::make_shared<NativeThreadLinux>(*this, thread_id);
   m_threads.push_back(thread_sp);
 
   if (m_pt_proces_trace_id != LLDB_INVALID_UID) {
diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
index 9584713d365..a06a6204c82 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -42,11 +42,11 @@ class NativeProcessLinux : public NativeProcessProtocol {
 public:
   class Factory : public NativeProcessProtocol::Factory {
   public:
-    llvm::Expected<NativeProcessProtocolSP>
+    llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
     Launch(ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
            MainLoop &mainloop) const override;
 
-    llvm::Expected<NativeProcessProtocolSP>
+    llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
     Attach(lldb::pid_t pid, NativeDelegate &native_delegate,
            MainLoop &mainloop) const override;
   };
@@ -160,15 +160,14 @@ private:
   // Private Instance Methods
   // ---------------------------------------------------------------------
   NativeProcessLinux(::pid_t pid, int terminal_fd, NativeDelegate &delegate,
-                     const ArchSpec &arch, MainLoop &mainloop);
+                     const ArchSpec &arch, MainLoop &mainloop,
+                     llvm::ArrayRef<::pid_t> tids);
 
   // Returns a list of process threads that we have attached to.
   static llvm::Expected<std::vector<::pid_t>> Attach(::pid_t pid);
 
   static Status SetDefaultPtraceOpts(const lldb::pid_t);
 
-  void InitializeThreads(llvm::ArrayRef<::pid_t> tids);
-
   void MonitorCallback(lldb::pid_t pid, bool exited, WaitStatus status);
 
   void WaitForNewThread(::pid_t tid);
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
index 43253f38801..30f09f0c3a3 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
@@ -30,11 +30,7 @@ lldb::ByteOrder NativeRegisterContextLinux::GetByteOrder() const {
   // read.
   lldb::ByteOrder byte_order = lldb::eByteOrderInvalid;
 
-  NativeProcessProtocolSP process_sp(m_thread.GetProcess());
-  if (!process_sp)
-    return byte_order;
-
-  if (!process_sp->GetByteOrder(byte_order)) {
+  if (!m_thread.GetProcess().GetByteOrder(byte_order)) {
     // FIXME log here
   }
 
diff --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
index b1d13668f32..5cd40941dcf 100644
--- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -85,7 +85,7 @@ void LogThreadStopInfo(Log &log, const ThreadStopInfo &stop_info,
 }
 }
 
-NativeThreadLinux::NativeThreadLinux(NativeProcessLinux *process,
+NativeThreadLinux::NativeThreadLinux(NativeProcessLinux &process,
                                      lldb::tid_t tid)
     : NativeThreadProtocol(process, tid), m_state(StateType::eStateInvalid),
       m_stop_info(), m_reg_context_sp(), m_stop_description() {}
@@ -144,12 +144,8 @@ NativeRegisterContextSP NativeThreadLinux::GetRegisterContext() {
   if (m_reg_context_sp)
     return m_reg_context_sp;
 
-  NativeProcessProtocolSP m_process_sp = m_process_wp.lock();
-  if (!m_process_sp)
-    return NativeRegisterContextSP();
-
   ArchSpec target_arch;
-  if (!m_process_sp->GetArchitecture(target_arch))
+  if (!m_process.GetArchitecture(target_arch))
     return NativeRegisterContextSP();
 
   const uint32_t concrete_frame_idx = 0;
@@ -460,20 +456,10 @@ void NativeThreadLinux::MaybeLogStateChange(lldb::StateType new_state) {
   if (new_state == old_state)
     return;
 
-  NativeProcessProtocolSP m_process_sp = m_process_wp.lock();
-  lldb::pid_t pid =
-      m_process_sp ? m_process_sp->GetID() : LLDB_INVALID_PROCESS_ID;
-
-  // Log it.
-  log->Printf("NativeThreadLinux: thread (pid=%" PRIu64 ", tid=%" PRIu64
-              ") changing from state %s to %s",
-              pid, GetID(), StateAsCString(old_state),
-              StateAsCString(new_state));
+  LLDB_LOG(log, "pid={0}, tid={1}: changing from state {2} to {3}",
+           m_process.GetID(), GetID(), old_state, new_state);
 }
 
 NativeProcessLinux &NativeThreadLinux::GetProcess() {
-  auto process_sp = std::static_pointer_cast<NativeProcessLinux>(
-      NativeThreadProtocol::GetProcess());
-  assert(process_sp);
-  return *process_sp;
+  return static_cast<NativeProcessLinux &>(m_process);
 }
diff --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.h b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
index b9126b3752a..54bec3d7750 100644
--- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
+++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
@@ -27,7 +27,7 @@ class NativeThreadLinux : public NativeThreadProtocol {
   friend class NativeProcessLinux;
 
 public:
-  NativeThreadLinux(NativeProcessLinux *process, lldb::tid_t tid);
+  NativeThreadLinux(NativeProcessLinux &process, lldb::tid_t tid);
 
   // ---------------------------------------------------------------------
   // NativeThreadProtocol Interface