[Lldb-commits] [lldb] r333781 - [lldb, process] Fix occasional hang when launching a process in LLDB

[Stella Stamenova via lldb-commits]

Author: stella.stamenova
Date: Fri Jun  1 12:14:53 2018
New Revision: 333781

URL: http://llvm.org/viewvc/llvm-project?rev=333781&view=rev
Log:
[lldb, process] Fix occasional hang when launching a process in LLDB

Summary:
Occasionally, when launching a process in lldb (especially on windows, but not limited to), lldb will hang before the process is launched and it will never recover. This happens because the timing of the processing of the state changes can be slightly different. The state changes that are issued are:

1) SetPublicState(eStateLaunching)
2) SetPrivateState(eStateLaunching)
3) SetPublicState(eStateStopped)
4) SetPrivateState(eStateStopped)

What we expect to see is:
public state: launching -> launching -> stopped
private state: launching -> stopped

What we see is:
public state: launching -> stopped -> launching
private state: launching -> stopped

The second launching change to the public state is issued when WaitForProcessStopPrivate calls HandlePrivateEvent on the event which was created when the private state was set to launching. HandlePrivateEvent has logic to determine whether to broadcase the event and a launching event is *always* broadcast. At the same time, when the stopped event is processed by WaitForProcessStopPrivate next, the function exists and that event is never broadcast, so the public state remains as launching.

HandlePrivateEvent does two things: determine whether there's a next action as well as determine whether to broadcast the event that was processed. There's only ever a next action set if we are trying to attach to a process, but WaitForProcessStopPrivate is only ever called when we are launching a process or connecting remotely, so the first part of HandlePrivateEvent (handling the next action) is irrelevant for WaitForProcessStopPrivate. As far as broadcasting the event is concerned, since we are handling state changes that already occurred to the public state (and are now duplicated in the private state), I believe the broadcast step is unnecessary also (and in fact, it causes the hang).

This change removes the call to HandlePrivateEvent from inside WaitForProcessStopPrivate.

Incidentally, there was also a bug filed recently that is the same issue: https://bugs.llvm.org/show_bug.cgi?id=37496

Reviewers: asmith, labath, zturner, jingham

Reviewed By: zturner, jingham

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D47609

Modified:
    lldb/trunk/include/lldb/lldb-enumerations.h
    lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
    lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/include/lldb/lldb-enumerations.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb-enumerations.h?rev=333781&r1=333780&r2=333781&view=diff
==============================================================================
--- lldb/trunk/include/lldb/lldb-enumerations.h (original)
+++ lldb/trunk/include/lldb/lldb-enumerations.h Fri Jun  1 12:14:53 2018
@@ -44,6 +44,9 @@ enum StateType {
                    ///launched or attached to anything yet
   eStateAttaching, ///< Process is currently trying to attach
   eStateLaunching, ///< Process is in the process of launching
+  // The state changes eStateAttaching and eStateLaunching are both sent while the
+  // private state thread is either not yet started or paused. For that reason, they
+  // should only be signaled as public state changes, and not private state changes.
   eStateStopped,   ///< Process or thread is stopped and can be examined.
   eStateRunning,   ///< Process or thread is running and can't be examined.
   eStateStepping,  ///< Process or thread is in the process of stepping and can

Modified: lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp?rev=333781&r1=333780&r2=333781&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp Fri Jun  1 12:14:53 2018
@@ -249,7 +249,6 @@ Status ProcessWindows::DoLaunch(Module *
   bool stop_at_entry = launch_info.GetFlags().Test(eLaunchFlagStopAtEntry);
   m_session_data.reset(new ProcessWindowsData(stop_at_entry));
 
-  SetPrivateState(eStateLaunching);
   DebugDelegateSP delegate(new LocalDebugDelegate(shared_from_this()));
   m_session_data->m_debugger.reset(new DebuggerThread(delegate));
   DebuggerThreadSP debugger = m_session_data->m_debugger;

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=333781&r1=333780&r2=333781&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Fri Jun  1 12:14:53 2018
@@ -2688,8 +2688,7 @@ StateType
 Process::WaitForProcessStopPrivate(EventSP &event_sp,
                                    const Timeout<std::micro> &timeout) {
   StateType state;
-  // Now wait for the process to launch and return control to us, and then call
-  // DidLaunch:
+
   while (true) {
     event_sp.reset();
     state = GetStateChangedEventsPrivate(event_sp, timeout);
@@ -2767,6 +2766,9 @@ Status Process::Launch(ProcessLaunchInfo
           }
         } else {
           EventSP event_sp;
+
+          // Now wait for the process to launch and return control to us, and then call
+          // DidLaunch:
           StateType state = WaitForProcessStopPrivate(event_sp, seconds(10));
 
           if (state == eStateInvalid || !event_sp) {