[Lldb-commits] [PATCH] D55631: Delay replacing the process till after we've finalized it

Greg Clayton via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Dec 13 09:41:13 PST 2018


clayborg added inline comments.


================
Comment at: source/Target/Target.cpp:2867-2880
     // Get a weak pointer to the previous process if we have one
     ProcessWP process_wp;
     if (m_process_sp)
       process_wp = m_process_sp;
-    m_process_sp =
-        GetPlatform()->DebugProcess(launch_info, debugger, this, error);
 
     // Cleanup the old process since someone might still have a strong
     // reference to this process and we would like to allow it to cleanup as
----------------
All this weak pointer code is no longer needed now? This code:

```
    // Get a weak pointer to the previous process if we have one
    ProcessWP process_wp;
    if (m_process_sp)
      process_wp = m_process_sp;



    // Cleanup the old process since someone might still have a strong
    // reference to this process and we would like to allow it to cleanup as
    // much as it can without the object being destroyed. We try to lock the
    // shared pointer and if that works, then someone else still has a strong
    // reference to the process.

    ProcessSP old_process_sp(process_wp.lock());
    if (old_process_sp)
      old_process_sp->Finalize();
```

Will just become:

```
if (m_process_sp)
  m_process_sp->Finalize();
```

The original thinking was that Process::Finalize() was just an optional call that could be made in case a process does not go away to reduce the memory that it owned in case it sticks around forever and also to mark the process in a way that would stop it from doing any future work (don't try to fetch thread lists, etc). Has Finalize changed to something that is required now? Are we doing work that must be done in order to properly shut down a process? That wasn't the original intention IIRC. "svn blame" was no help as all this code was there prior the great reformatting of code.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55631/new/

https://reviews.llvm.org/D55631





More information about the lldb-commits mailing list