[Lldb-commits] [PATCH] D69503: [LLDB] [Windows] Don't crash if the debugged process is unable to start up
Adrian McCarthy via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Wed Oct 30 16:17:00 PDT 2019
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.
CHANGES SINCE LAST ACTION
More information about the lldb-commits