[Lldb-commits] [PATCH] Prevent LLGS from crashing when exiting - make NativeProcessLinux to wait until ThreadStateCoordinator is fully stopped before entering ~NativeProcessLinux.

[Oleksiy Vyalov]

On Tue, Feb 17, 2015 at 10:01 AM, Tamas Berghammer <tberghammer at google.com>
wrote:

> 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.
>
>
Yes, an instance is functional when its destructor is being called but
there some limitations:

   - You may hit undefined behavior if virtual function are called when
   destruction is in progress -
   http://www.artima.com/cppsource/nevercall.html, i.e. if some virtual
   function of NativeProcessLinux are called by TSC when ~NativeProcessLinux
   is in progress - it might be a problem.
   - 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 -
   https://github.com/llvm-mirror/lldb/blob/master/include/lldb/Host/common/NativeThreadProtocol.h#L77.
   NativeThreadProtocol::GetProcess gives you a locked NativeProcessProtocolSP
   (or (NativeProcessProtocolSP(nullptr) if shared_ptr is no longer alive)).

           Here the situation which I faced when I added just
m_coordinator_thread.Join (nullptr); without Terminate call:

   - ~NativeProcessLinux is calling StopMonitor
      - NativeThreadLinux::SetRunning is called from TSC thread -
      https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/Process/Linux/NativeProcessLinux.cpp#L2141
      - NativeThreadLinux::SetRunning is trying to get process shared_ptr
      via GetProcess()
      - 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)
      - SIGSEGV when trying to call GetProcess()->GetWatchpointMap();





> I am confused if the introduction of the Terminate method made any
> difference.
>
>
  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.


> http://reviews.llvm.org/D7692
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>


-- 
Oleksiy Vyalov | Software Engineer | ovyalov at google.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150217/609c3b36/attachment.html>