amccarth requested changes to this revision.
amccarth added a comment.
This revision now requires changes to proceed.

Thanks for identifying this race condition.

After some poking around, I think there's a cleaner way to address it.

First, `m_session_state`'s lifetime is currently managed mostly by 
`ProcessDebugger` (base class) but for one exception in `ProcessWindows` 
(derived class).  There doesn't seem to be a good reason for that 
inconsistency, so my first step was to eliminate it and make `ProcessDebugger` 
responsible for its lifetime in all cases. This is done by moving the 
`m_session_state.reset()` to `ProcessDebugger::OnExitProcess` and having 
`ProcessWindows::OnExitProcess` call the other.  This has the nice additional 
benefit of eliminating some duplicate code and comments.

But I'm not sure we need to clear `m_session_state` even in 
`ProcessDebugger::OnExitProcess`.  There are two places where `m_session_state` 
is created:  `ProcessDebugger::AttachProcess` and 
`ProcessDebugger::LaunchProcess`.  We could put `m_session_state.reset()`s in 
those functions, where it handles failure of `WaitForDebuggerConnection`.  But 
I'm not even sure if _that_ is necessary.

By adding a virtual destructor to the base class, it seems everything cleans up 
naturally before starting the next debugging session.

Here's what it would look like:  https://reviews.llvm.org/P8168

(If I'm wrong, and it _is_ important for us to clear the session state 
explicitly, then we'd need to find a place after `OnExitProcess`.  I don't 
think such a hook exists right now, so perhaps we'd have to create one.  That 
hook could be the one place we reset `m_session_state` for both the successful 
and unsuccessful debug sessions.)

FYI:  I'll be offline until November 4.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69503



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to