[Lldb-commits] [lldb] r223083 - lldb can deadlock when launched with an non-existing executable:

[Greg Clayton]

Author: gclayton
Date: Mon Dec  1 16:41:27 2014
New Revision: 223083

URL: http://llvm.org/viewvc/llvm-project?rev=223083&view=rev
Log:
lldb can deadlock when launched with an non-existing executable:

% lldb /bin/nonono
(lldb) target create "/bin/nonono"
error: unable to find executable for '/usr/bin/nonono'
<deadlock>

The problem was the initial commands 'target create "/bin/nonono"' were put into a pipe and the command interpreter was being run with:

void
CommandInterpreter::RunCommandInterpreter(bool auto_handle_events,
                                          bool spawn_thread,
                                          CommandInterpreterRunOptions &options)
{
    // Always re-create the command intepreter when we run it in case
    // any file handles have changed.
    bool force_create = true;
    m_debugger.PushIOHandler(GetIOHandler(force_create, &options));
    m_stopped_for_crash = false;
    
    if (auto_handle_events)
        m_debugger.StartEventHandlerThread();
    
    if (spawn_thread)
    {
        m_debugger.StartIOHandlerThread();
    }
    else
    {
        m_debugger.ExecuteIOHanders();
        
        if (auto_handle_events)
            m_debugger.StopEventHandlerThread();
    }
    
}

If "auto_handle_events" was set to true and "spawn_thread" was false, we would execute:

m_debugger.StartEventHandlerThread();
m_debugger.ExecuteIOHanders();
m_debugger.StopEventHandlerThread();


The problem was there was no synchonization in Debugger::StartEventHandlerThread() to ensure the event handler was listening to events and the the call to "m_debugger.StopEventHandlerThread()" would do:

void
Debugger::StopEventHandlerThread()
{
    if (m_event_handler_thread.IsJoinable())
    {
        GetCommandInterpreter().BroadcastEvent(CommandInterpreter::eBroadcastBitQuitCommandReceived);
        m_event_handler_thread.Join(nullptr);
    }
}

The problem was that the event thread might not be listening for the CommandInterpreter::eBroadcastBitQuitCommandReceived event yet.

The solution is to make sure the Debugger::DefaultEventHandler() is listening to events before we return from Debugger::StartEventHandlerThread(). Once we have this synchonization we remove the race condition.

This fixes radar:

<rdar://problem/19041192>



Modified:
    lldb/trunk/include/lldb/Core/Debugger.h
    lldb/trunk/source/Core/Debugger.cpp

Modified: lldb/trunk/include/lldb/Core/Debugger.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Debugger.h?rev=223083&r1=223082&r2=223083&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/Debugger.h (original)
+++ lldb/trunk/include/lldb/Core/Debugger.h Mon Dec  1 16:41:27 2014
@@ -431,6 +431,10 @@ protected:
     {
         return m_source_file_cache;
     }
+
+    void
+    InstanceInitialize ();
+
     lldb::StreamFileSP m_input_file_sp;
     lldb::StreamFileSP m_output_file_sp;
     lldb::StreamFileSP m_error_file_sp;
@@ -454,10 +458,17 @@ protected:
     LoadedPluginsList m_loaded_plugins;
     HostThread m_event_handler_thread;
     HostThread m_io_handler_thread;
+    Broadcaster m_sync_broadcaster;
     lldb::ListenerSP m_forward_listener_sp;
-    void
-    InstanceInitialize ();
-    
+
+    //----------------------------------------------------------------------
+    // Events for m_sync_broadcaster
+    //----------------------------------------------------------------------
+    enum
+    {
+        eBroadcastBitEventThreadIsListening   = (1 << 0),
+    };
+
 private:
 
     // Use Debugger::CreateInstance() to get a shared pointer to a new

Modified: lldb/trunk/source/Core/Debugger.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Debugger.cpp?rev=223083&r1=223082&r2=223083&view=diff
==============================================================================
--- lldb/trunk/source/Core/Debugger.cpp (original)
+++ lldb/trunk/source/Core/Debugger.cpp Mon Dec  1 16:41:27 2014
@@ -661,7 +661,10 @@ Debugger::Debugger(lldb::LogOutputCallba
     m_command_interpreter_ap(new CommandInterpreter(*this, eScriptLanguageDefault, false)),
     m_input_reader_stack(),
     m_instance_name(),
-    m_loaded_plugins()
+    m_loaded_plugins(),
+    m_event_handler_thread (),
+    m_io_handler_thread (),
+    m_sync_broadcaster (NULL, "lldb.debugger.sync")
 {
     char instance_cstr[256];
     snprintf(instance_cstr, sizeof(instance_cstr), "debugger_%d", (int)GetID());
@@ -3250,17 +3253,14 @@ Debugger::DefaultEventHandler()
                                       CommandInterpreter::eBroadcastBitQuitCommandReceived      |
                                       CommandInterpreter::eBroadcastBitAsynchronousOutputData   |
                                       CommandInterpreter::eBroadcastBitAsynchronousErrorData    );
-    
+
+    // Let the thread that spawned us know that we have started up and
+    // that we are now listening to all required events so no events get missed
+    m_sync_broadcaster.BroadcastEvent(eBroadcastBitEventThreadIsListening);
+
     bool done = false;
     while (!done)
     {
-//        Mutex::Locker locker;
-//        if (locker.TryLock(m_input_reader_stack.GetMutex()))
-//        {
-//            if (m_input_reader_stack.IsEmpty())
-//                break;
-//        }
-//
         EventSP event_sp;
         if (listener.WaitForEvent(NULL, event_sp))
         {
@@ -3344,9 +3344,25 @@ Debugger::StartEventHandlerThread()
 {
     if (!m_event_handler_thread.IsJoinable())
     {
+        // We must synchronize with the DefaultEventHandler() thread to ensure
+        // it is up and running and listening to events before we return from
+        // this function. We do this by listening to events for the
+        // eBroadcastBitEventThreadIsListening from the m_sync_broadcaster
+        Listener listener("lldb.debugger.event-handler");
+        listener.StartListeningForEvents(&m_sync_broadcaster, eBroadcastBitEventThreadIsListening);
+
         // Use larger 8MB stack for this thread
-        m_event_handler_thread = ThreadLauncher::LaunchThread("lldb.debugger.event-handler", EventHandlerThread, this, NULL,
+        m_event_handler_thread = ThreadLauncher::LaunchThread("lldb.debugger.event-handler", EventHandlerThread,
+                                                              this,
+                                                              NULL,
                                                               g_debugger_event_thread_stack_bytes);
+
+        // Make sure DefaultEventHandler() is running and listening to events before we return
+        // from this function. We are only listening for events of type
+        // eBroadcastBitEventThreadIsListening so we don't need to check the event, we just need
+        // to wait an infinite amount of time for it (NULL timeout as the first parameter)
+        lldb::EventSP event_sp;
+        listener.WaitForEvent(NULL, event_sp);
     }
     return m_event_handler_thread.IsJoinable();
 }