[Lldb-commits] [PATCH] D69503: [LLDB] [Windows] Don't crash if the debugged process is unable to start up

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

Yes, this definitely is a race condition. Without any changes, I triggered this condition on arm64, but not on x86_64.

The issue is in the subclass ProcessWindows, where OnExitProcess does this:

  void ProcessWindows::OnDebuggerError(const Status &error, uint32_t type) {
    // [simplified]
    if (m_session_data->m_initial_stop_received) {
    } else {
      // If we haven't actually launched the process yet, this was an error
      // launching the process.  Set the internal error and signal the initial
      // stop event so that the DoLaunch method wakes up and returns a failure.
      m_session_data->m_launch_error = error;
      ::SetEvent(m_session_data->m_initial_stop_event);
      return;
    }
  }   
  
  void ProcessWindows::OnExitProcess(uint32_t exit_code) {
    // [irrelevant bits excluded]
  
    // If the process exits before any initial stop then notify the debugger
    // of the error otherwise WaitForDebuggerConnection() will be blocked.
    // An example of this issue is when a process fails to load a dependent DLL.
    if (m_session_data && !m_session_data->m_initial_stop_received) {
      Status error(exit_code, eErrorTypeWin32);
      OnDebuggerError(error, 0);
    }
  
    // Reset the session.
    m_session_data.reset();
  }

So `ProcessWindows::OnExitProcess` signals to `ProcessDebugger::WaitForDebuggerConnection` to proceed, and then `ProcessWindows::OnExitProcess` resets `m_session_data`, which `WaitForDebuggerConnection` starts inspecting.

What's the correct course of action here? Remove the `m_session_data.reset()` from ProcessWindows::OnExitProcess, concluding that it's up to some other class to clear `m_session_data` in this case?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69503