<div dir="ltr">Thanks for the explanation (especially for the call stack). Now I see why your change fix the issue and it looks good, but I still don't like it. Possibly I would suggest to check for nullptr in the SetRunning function as it is dereferencing a pointer (returned by GetProcess) what can be nullptr, but I am not sure if it is better or not.<div><br></div><div>P.S.: Calling a virtual function in a destructor is almost never undefined behavior (it is not UB here) just it have surprising effect, and this is why most book suggest not to do it.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 17, 2015 at 6:44 PM, Oleksiy Vyalov <span dir="ltr"><<a href="mailto:ovyalov@google.com" target="_blank">ovyalov@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Tue, Feb 17, 2015 at 10:01 AM, Tamas Berghammer <span dir="ltr"><<a href="mailto:tberghammer@google.com" target="_blank">tberghammer@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">As far as I know NativeProcessLinux is still fully functional during the execution of it's destructor. The only difference is that the members in GDBRemoteCommunicationServerLLGS defined after the NativeProcessLinux smart pointer are already destructed. If this ordering is the problem then it can be solved with changing the order of the member declarations inside GDBRemoteCommunicationServerLLGS.<br>
<br></blockquote><div><br></div></span><div>Yes, an instance is functional when its destructor is being called but there some limitations:</div><div><ul><li>You may hit undefined behavior if virtual function are called when destruction is in progress - <a href="http://www.artima.com/cppsource/nevercall.html" target="_blank">http://www.artima.com/cppsource/nevercall.html</a>, i.e. if some virtual function of NativeProcessLinux are called by TSC when ~NativeProcessLinux is in progress - it might be a problem.<br></li><li>NativeProcessLinux is derived from NativeProcessProtocol which in its turn holds list of threads - "std::vector<NativeThreadProtocolSP> m_threads;". Each instance of NativeThreadProtocol maintains a weak pointer to its process - <a href="https://github.com/llvm-mirror/lldb/blob/master/include/lldb/Host/common/NativeThreadProtocol.h#L77" target="_blank">https://github.com/llvm-mirror/lldb/blob/master/include/lldb/Host/common/NativeThreadProtocol.h#L77</a>. NativeThreadProtocol::GetProcess gives you a locked NativeProcessProtocolSP (or (NativeProcessProtocolSP(nullptr) if shared_ptr is no longer alive)). </li></ul><div> Here the situation which I faced when I added just m_coordinator_thread.Join (nullptr); without Terminate call:</div><div><ul><ul><li>~NativeProcessLinux is calling StopMonitor</li><li>NativeThreadLinux::SetRunning is called from TSC thread - <a href="https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/Process/Linux/NativeProcessLinux.cpp#L2141" target="_blank">https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/Process/Linux/NativeProcessLinux.cpp#L2141</a></li><li>NativeThreadLinux::SetRunning is trying to get process shared_ptr via GetProcess()<br></li><li>Since GDBRemoteCommunicationServerLLGS holds lldb_private::NativeProcessProtocolSP m_debugged_process_sp; and we're inside of ~NativeProcessLinux - so, shared_ptr is no longer valid and GetProcess() returns NativeProcessProtocolSP(nullptr)</li><li>SIGSEGV when trying to call GetProcess()->GetWatchpointMap();</li></ul></ul></div></div><span class=""><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
I am confused if the introduction of the Terminate method made any difference.<br>
<div><div><br></div></div></blockquote><div><br></div></span><div> I assume we may put more safety nets into NativeThreadProtocol::GetProcess - add more assertations to check value of shared_ptr or check return value of GetProcess for nullptr. But to be sure that NativeProcessLinux is still accessible via shared_ptr and virtual methods can be safely executed on NativeProcessLinux I decided to introduce Terminate method. </div><span class=""><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>
<br>
<a href="http://reviews.llvm.org/D7692" target="_blank">http://reviews.llvm.org/D7692</a><br>
<br>
EMAIL PREFERENCES<br>
<a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</div></div></blockquote></span></div><span class="HOEnZb"><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr"><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(213,15,37);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)">Oleksiy Vyalov |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(51,105,232);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)"> Software Engineer |</span><span style="color:rgb(85,85,85);font-family:sans-serif;line-height:20px;border-width:2px 0px 0px;border-style:solid;border-color:rgb(0,153,57);padding-top:2px;margin-top:2px;background-color:rgb(255,255,255)"> <a href="mailto:ovyalov@google.com" target="_blank">ovyalov<font color="#1155cc">@google.com</font></a></span></div></div>
</font></span></div></div>
</blockquote></div><br></div>