From e24c4acf6cd5ecbfed993c3b598dbc2fe24ab7ab Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Thu, 17 Nov 2011 04:46:02 +0000 Subject: [PATCH] Fixed the issue that was causing our monitor process threads to crash, it turned out to be unitialized data in the ProcessLaunchInfo default constructor. Turning on MallocScribble in the environment helped track this down. When we launch and attach using the host layer, we now inform the process that it shouldn't detach when by calling an accessor. llvm-svn: 144882 --- lldb/include/lldb/Target/Process.h | 24 +++++++++++--- lldb/source/Core/Debugger.cpp | 2 +- .../Process/MacOSX-Kernel/ProcessKDP.cpp | 6 +++- .../Process/gdb-remote/ProcessGDBRemote.cpp | 3 ++ lldb/source/Target/Platform.cpp | 8 +++++ lldb/source/Target/Process.cpp | 33 +++++++++++++++++-- 6 files changed, 67 insertions(+), 9 deletions(-) diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 44262e52ba16..2528818e85d7 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -491,7 +491,15 @@ public: ProcessLaunchInfo () : ProcessInfo(), - m_flags () + m_working_dir (), + m_plugin_name (), + m_shell (), + m_flags (0), + m_file_actions (), + m_resume_count (0), + m_monitor_callback (NULL), + m_monitor_callback_baton (NULL), + m_monitor_signals (false) { } @@ -1508,11 +1516,17 @@ public: ConnectRemote (const char *remote_url); bool - AttachedToProcess() const + GetShouldDetach () const { - return m_attached_to_process; + return m_should_detach; } - + + void + SetShouldDetach (bool b) + { + m_should_detach = b; + } + //------------------------------------------------------------------ /// Get the image information address for the current process. /// @@ -2944,7 +2958,7 @@ protected: std::string m_stderr_data; MemoryCache m_memory_cache; AllocatedMemoryCache m_allocated_memory_cache; - bool m_attached_to_process; /// Did we launch the process or attach to it? + bool m_should_detach; /// Should we detach if the process object goes away with an explicit call to Kill or Detach? typedef std::map LanguageRuntimeCollection; LanguageRuntimeCollection m_language_runtimes; diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 6f95bd7a276d..e2dd2e56bd3b 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -272,7 +272,7 @@ Debugger::Clear() ProcessSP process_sp (m_target_list.GetTargetAtIndex (i)->GetProcessSP()); if (process_sp) { - if (process_sp->AttachedToProcess()) + if (process_sp->GetShouldDetach()) process_sp->Detach(); else process_sp->Destroy(); diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp index df01e94cb143..fed40e0ff1bb 100644 --- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp +++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp @@ -97,6 +97,11 @@ ProcessKDP::ProcessKDP(Target& target, Listener &listener) : ProcessKDP::~ProcessKDP() { Clear(); + // We need to call finalize on the process before destroying ourselves + // to make sure all of the broadcaster cleanup goes as planned. If we + // destruct this class, then Process::~Process() might have problems + // trying to fully destroy the broadcaster. + Finalize(); } //---------------------------------------------------------------------- @@ -620,7 +625,6 @@ ProcessKDP::DisableWatchpoint (Watchpoint *wp) void ProcessKDP::Clear() { - Mutex::Locker locker (m_thread_list.GetMutex ()); m_thread_list.Clear(); } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 1aa3c920a08d..95d452f65bb7 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1171,6 +1171,9 @@ ProcessGDBRemote::SetThreadStopInfo (StringExtractor& stop_packet) { // thread in big endian hex tid = Args::StringToUInt32 (value.c_str(), 0, 16); + // m_thread_list does have its own mutex, but we need to + // hold onto the mutex between the call to m_thread_list.FindThreadByID(...) + // and the m_thread_list.AddThread(...) so it doesn't change on us Mutex::Locker locker (m_thread_list.GetMutex ()); thread_sp = m_thread_list.FindThreadByID(tid, false); if (!thread_sp) diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp index 3b7804288368..dbd4e4058093 100644 --- a/lldb/source/Target/Platform.cpp +++ b/lldb/source/Target/Platform.cpp @@ -581,6 +581,14 @@ Platform::DebugProcess (ProcessLaunchInfo &launch_info, { ProcessAttachInfo attach_info (launch_info); process_sp = Attach (attach_info, debugger, target, listener, error); + if (process_sp) + { + // Since we attached to the process, it will think it needs to detach + // if the process object just goes away without an explicit call to + // Process::Kill() or Process::Detach(), so let it know to kill the + // process if this happens. + process_sp->SetShouldDetach (false); + } } } return process_sp; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 3d917a6f7587..d132eb2f69d1 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -774,7 +774,7 @@ Process::Process(Target &target, Listener &listener) : m_stderr_data (), m_memory_cache (*this), m_allocated_memory_cache (*this), - m_attached_to_process (false), + m_should_detach (false), m_next_event_action_ap(), m_can_jit(eCanJITYes) { @@ -818,6 +818,29 @@ Process::~Process() void Process::Finalize() { + switch (GetPrivateState()) + { + case eStateConnected: + case eStateAttaching: + case eStateLaunching: + case eStateStopped: + case eStateRunning: + case eStateStepping: + case eStateCrashed: + case eStateSuspended: + if (GetShouldDetach()) + Detach(); + else + Destroy(); + break; + + case eStateInvalid: + case eStateUnloaded: + case eStateDetached: + case eStateExited: + break; + } + // Clear our broadcaster before we proceed with destroying Broadcaster::Clear(); @@ -1183,6 +1206,9 @@ Process::UpdateThreadListIfNeeded () if (StateIsStoppedState (state, true)) { Mutex::Locker locker (m_thread_list.GetMutex ()); + // m_thread_list does have its own mutex, but we need to + // hold onto the mutex between the call to UpdateThreadList(...) + // and the os->UpdateThreadList(...) so it doesn't change on us ThreadList new_thread_list(this); // Always update the thread list with the protocol specific // thread list @@ -2209,6 +2235,7 @@ Process::Launch (const ProcessLaunchInfo &launch_info) if (error.Success()) { SetPublicState (eStateLaunching); + m_should_detach = false; // Now launch using these arguments. error = DoLaunch (exe_module, launch_info); @@ -2351,6 +2378,8 @@ Process::Attach (ProcessAttachInfo &attach_info) error = WillAttachToProcessWithName(process_name, wait_for_launch); if (error.Success()) { + m_should_detach = true; + SetPublicState (eStateAttaching); error = DoAttachToProcessWithName (process_name, wait_for_launch); if (error.Fail()) @@ -2416,6 +2445,7 @@ Process::Attach (ProcessAttachInfo &attach_info) error = WillAttachToProcessWithID(attach_pid); if (error.Success()) { + m_should_detach = true; SetPublicState (eStateAttaching); error = DoAttachToProcessWithID (attach_pid); @@ -2516,7 +2546,6 @@ Process::CompleteAttach () { // Let the process subclass figure out at much as it can about the process // before we go looking for a dynamic loader plug-in. - m_attached_to_process = true; DidAttach(); // We just attached. If we have a platform, ask it for the process architecture, and if it isn't