From afa91e339ba053828e352fe1509c8d415d91a591 Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Mon, 1 Dec 2014 22:41:27 +0000 Subject: [PATCH] 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' 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: llvm-svn: 223083 --- lldb/include/lldb/Core/Debugger.h | 17 ++++++++++++++--- lldb/source/Core/Debugger.cpp | 36 ++++++++++++++++++++++++++---------- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index e02ae9f..9a68376 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -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 diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 98fb64c..6cb844f2 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -661,7 +661,10 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, void *baton) : 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(); } -- 2.7.4