Don't call the Process::SyncIOHandler in Target::Launch

[ki.stfu]

Summary

This patch moves synchronization of iohandler to CommandObjectProcessLaunch::DoExecute like it was done in CommandObjectProcessContinue::DoExecute.

[ki.stfu]

Friendly ping.

[clayborg]

Looks good.

[labath]

Is change this necessary to ensure correctness, or is it just a matter of saving some lines of code?

I am asking this because, this change has increased the running time of the test suite (compare http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/2252 with http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/2251), sending TestConcurrentEvents over the 5m limit on the linux build bot. It's not clear to me why is this the case, but the test takes ~6 minutes now, where previously it took slightly less than 5. For now I have increased the timeout limit to address this, but we will need to do something different in the future.

[clayborg]

My IOHandler patch will eventually fix this, but for now any calls to process_sp->SyncIOHandler(2000) end up causing me to sleep for 2 seconds since the SyncIOHandler is essentially a sleep(2) as it is not doing what the original author intended it to do. This could be fixed in a separate patch until we can truly get this fixed. The issue with SyncIOHandler() is it does:

m_iohandler_sync.WaitForValueEqualTo(true, &timeout, &timed_out);

where "timeout" is set to 2000 ms...

But the problem with quick continues where you hit a breakpoint right away is there is code that does:

void
Process::HandlePrivateEvent (EventSP &event_sp)
{
    ....
    if (StateIsRunningState (new_state))
    {
        ....
        m_iohandler_sync.SetValue(true, eBroadcastAlways);
    }
    else if (StateIsStoppedState(new_state, false))
    {
        m_iohandler_sync.SetValue(false, eBroadcastNever);
    }
    ....

So a quick run will set m_iohandler_sync to true, then false. Then whomever calls process_sp->SyncIOHandler(...) will block for 2000 ms since m_iohandler_sync is set to true, then false again. So you could basically just replace all m_iohandler_sync.SetValue(2000) calls with sleep(2) and you would get the same behavior as what we currently have. This is what I was trying to fix with my IOHandler patches, but they didn't work for everyone. We really need the linux folks and others to help improve my IOHandler patch that removes the SyncIOHandler() and coordinates async output with the top IOHandler to truly fix this.

[ki.stfu]

RE: Is change this necessary to ensure correctness, or is it just a matter

of saving some lines of code?
It was done to ensure correctness because Target::Launch consists of ~4
branches of code where it was needed and it was to difficult to maintain.
In addition it is needed only for lldb to ensure that event is handled
before printing of (lldb) prompt, but if it doesn't matter when we using a
public API (i.e. SBTarget::Launch).
And the last benefit is to remove the duplicated code.

RE: It's not clear to me why is this the case, but the test takes ~6

minutes now, where previously it took slightly less than 5.
I think it is a matter of TestMiStartupOptoins failure because each test is
waiting ~30sec before failing.

Thanks,
Ilia

source/Commands/CommandObjectProcess.cpp

DoExecute (Args& launch_args, CommandReturnObject &result)
             target->SetRunArguments (launch_args);
         }
 
         StreamString stream;
         Error error = target->Launch(m_options.launch_info, &stream);
         
         if (error.Success())
         {
-            const char *archname = exe_module_sp->GetArchitecture().GetArchitectureName();
             ProcessSP process_sp (target->GetProcessSP());
             if (process_sp)
             {
+                // There is a race condition where this thread will return up the call stack to the main command
+                // handler and show an (lldb) prompt before HandlePrivateEvent (from PrivateStateThread) has
+                // a chance to call PushProcessIOHandler().
+                process_sp->SyncIOHandler(2000);
+
                 const char *data = stream.GetData();
                 if (data && strlen(data) > 0)
                     result.AppendMessage(stream.GetData());
+                const char *archname = exe_module_sp->GetArchitecture().GetArchitectureName();
                 result.AppendMessageWithFormat ("Process %" PRIu64 " launched: '%s' (%s)\n", process_sp->GetID(), exe_module_sp->GetFileSpec().GetPath().c_str(), archname);
                 result.SetStatus (eReturnStatusSuccessFinishResult);
                 result.SetDidChangeProcessState (true);
             }
             else
             {
                 result.AppendError("no error returned from Target::Launch, and target has no process");
                 result.SetStatus (eReturnStatusFailed);

source/Target/Target.cpp

if (error.Success())
                                 error.SetErrorStringWithFormat("process isn't stopped: %s", StateAsCString(state));
                             }
                         }
                     }
                     else
                     {
                         m_process_sp->RestoreProcessEvents();
                         error = m_process_sp->PrivateResume();
-                        if (error.Success())
-                        {
-                            // there is a race condition where this thread will return up the call stack to the main command
-                            // handler and show an (lldb) prompt before HandlePrivateEvent (from PrivateStateThread) has
-                            // a chance to call PushProcessIOHandler()
-                            m_process_sp->SyncIOHandler(2000);
-                        }
                     }
                     if (!error.Success())
                     {
                         Error error2;
                         error2.SetErrorStringWithFormat("process resume at entry point failed: %s", error.AsCString());
                         error = error2;
                     }
                 }
                 else
                 {
                     assert(synchronous_execution && launch_info.GetFlags().Test(eLaunchFlagStopAtEntry));
 
                     // Target was stopped at entry as was intended. Need to notify the listeners about it.
                     m_process_sp->RestoreProcessEvents();
                     m_process_sp->HandlePrivateEvent(event_sp);
-
-                    // there is a race condition where this thread will return up the call stack to the main command
-                    // handler and show an (lldb) prompt before HandlePrivateEvent (from PrivateStateThread) has
-                    // a chance to call PushProcessIOHandler()
-                    m_process_sp->SyncIOHandler(2000);
                 }
             }
             else if (state == eStateExited)
             {
                 bool with_shell = !!launch_info.GetShell();
                 const int exit_status = m_process_sp->GetExitStatus();
                 const char *exit_desc = m_process_sp->GetExitDescription();
 #define LAUNCH_SHELL_MESSAGE "\n'r' and 'run' are aliases that default to launching through a shell.\nTry launching without going through a shell by using 'process launch'."