[Lldb-commits] [lldb] r246756 - Fix deadlock while attaching to inferiors
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 3 02:36:23 PDT 2015
Author: labath
Date: Thu Sep 3 04:36:22 2015
New Revision: 246756
URL: http://llvm.org/viewvc/llvm-project?rev=246756&view=rev
Log:
Fix deadlock while attaching to inferiors
Summary:
There was a race condition in the AsyncThread, where we would end up sending a vAttach
notification to the thread before it got a chance set up its listener (this can be reproduced by
adding a sleep() at the very beginning of ProcessGDBRemote::AsyncThread()). This event would then
get lost and we LLDB would deadlock. I fix this by setting up the listener early on, in the
ProcessGDBRemote constructor.
This should improve the stability of all attach tests. For now, I am removing XTIMEOUT from
TestAttachResume, and will watch the buildbots for signs of trouble.
Reviewers: clayborg, ovyalov
Subscribers: lldb-commits
Differential Revision: http://reviews.llvm.org/D12552
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/trunk/test/dosep.py
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=246756&r1=246755&r2=246756&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Thu Sep 3 04:36:22 2015
@@ -385,6 +385,7 @@ ProcessGDBRemote::ProcessGDBRemote(lldb:
m_last_stop_packet_mutex (Mutex::eMutexTypeRecursive),
m_register_info (),
m_async_broadcaster (NULL, "lldb.process.gdb-remote.async-broadcaster"),
+ m_async_listener("lldb.process.gdb-remote.async-listener"),
m_async_thread_state_mutex(Mutex::eMutexTypeRecursive),
m_thread_ids (),
m_jstopinfo_sp (),
@@ -406,6 +407,25 @@ ProcessGDBRemote::ProcessGDBRemote(lldb:
m_async_broadcaster.SetEventName (eBroadcastBitAsyncThreadShouldExit, "async thread should exit");
m_async_broadcaster.SetEventName (eBroadcastBitAsyncContinue, "async thread continue");
m_async_broadcaster.SetEventName (eBroadcastBitAsyncThreadDidExit, "async thread did exit");
+
+ Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_ASYNC));
+
+ const uint32_t async_event_mask = eBroadcastBitAsyncContinue | eBroadcastBitAsyncThreadShouldExit;
+
+ if (m_async_listener.StartListeningForEvents(&m_async_broadcaster, async_event_mask) != async_event_mask)
+ {
+ if (log)
+ log->Printf("ProcessGDBRemote::%s failed to listen for m_async_broadcaster events", __FUNCTION__);
+ }
+
+ const uint32_t gdb_event_mask = Communication::eBroadcastBitReadThreadDidExit |
+ GDBRemoteCommunication::eBroadcastBitGdbReadThreadGotNotify;
+ if (m_async_listener.StartListeningForEvents(&m_gdb_comm, gdb_event_mask) != gdb_event_mask)
+ {
+ if (log)
+ log->Printf("ProcessGDBRemote::%s failed to listen for m_gdb_comm events", __FUNCTION__);
+ }
+
const uint64_t timeout_seconds = GetGlobalPluginProperties()->GetPacketTimeout();
if (timeout_seconds > 0)
m_gdb_comm.SetPacketTimeout(timeout_seconds);
@@ -3761,184 +3781,174 @@ ProcessGDBRemote::AsyncThread (void *arg
if (log)
log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") thread starting...", __FUNCTION__, arg, process->GetID());
- Listener listener ("ProcessGDBRemote::AsyncThread");
EventSP event_sp;
- const uint32_t desired_event_mask = eBroadcastBitAsyncContinue |
- eBroadcastBitAsyncThreadShouldExit;
-
- if (listener.StartListeningForEvents (&process->m_async_broadcaster, desired_event_mask) == desired_event_mask)
+ bool done = false;
+ while (!done)
{
- listener.StartListeningForEvents (&process->m_gdb_comm, Communication::eBroadcastBitReadThreadDidExit |
- GDBRemoteCommunication::eBroadcastBitGdbReadThreadGotNotify);
-
- bool done = false;
- while (!done)
+ if (log)
+ log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") listener.WaitForEvent (NULL, event_sp)...", __FUNCTION__, arg, process->GetID());
+ if (process->m_async_listener.WaitForEvent (NULL, event_sp))
{
- if (log)
- log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") listener.WaitForEvent (NULL, event_sp)...", __FUNCTION__, arg, process->GetID());
- if (listener.WaitForEvent (NULL, event_sp))
+ const uint32_t event_type = event_sp->GetType();
+ if (event_sp->BroadcasterIs (&process->m_async_broadcaster))
{
- const uint32_t event_type = event_sp->GetType();
- if (event_sp->BroadcasterIs (&process->m_async_broadcaster))
+ if (log)
+ log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") Got an event of type: %d...", __FUNCTION__, arg, process->GetID(), event_type);
+
+ switch (event_type)
{
- if (log)
- log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") Got an event of type: %d...", __FUNCTION__, arg, process->GetID(), event_type);
+ case eBroadcastBitAsyncContinue:
+ {
+ const EventDataBytes *continue_packet = EventDataBytes::GetEventDataFromEvent(event_sp.get());
- switch (event_type)
- {
- case eBroadcastBitAsyncContinue:
+ if (continue_packet)
{
- const EventDataBytes *continue_packet = EventDataBytes::GetEventDataFromEvent(event_sp.get());
+ const char *continue_cstr = (const char *)continue_packet->GetBytes ();
+ const size_t continue_cstr_len = continue_packet->GetByteSize ();
+ if (log)
+ log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") got eBroadcastBitAsyncContinue: %s", __FUNCTION__, arg, process->GetID(), continue_cstr);
- if (continue_packet)
+ if (::strstr (continue_cstr, "vAttach") == NULL)
+ process->SetPrivateState(eStateRunning);
+ StringExtractorGDBRemote response;
+
+ // If in Non-Stop-Mode
+ if (process->GetTarget().GetNonStopModeEnabled())
{
- const char *continue_cstr = (const char *)continue_packet->GetBytes ();
- const size_t continue_cstr_len = continue_packet->GetByteSize ();
- if (log)
- log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") got eBroadcastBitAsyncContinue: %s", __FUNCTION__, arg, process->GetID(), continue_cstr);
-
- if (::strstr (continue_cstr, "vAttach") == NULL)
- process->SetPrivateState(eStateRunning);
- StringExtractorGDBRemote response;
-
- // If in Non-Stop-Mode
- if (process->GetTarget().GetNonStopModeEnabled())
+ // send the vCont packet
+ if (!process->GetGDBRemote().SendvContPacket(process, continue_cstr, continue_cstr_len, response))
{
- // send the vCont packet
- if (!process->GetGDBRemote().SendvContPacket(process, continue_cstr, continue_cstr_len, response))
- {
- // Something went wrong
- done = true;
- break;
- }
+ // Something went wrong
+ done = true;
+ break;
}
- // If in All-Stop-Mode
- else
- {
- StateType stop_state = process->GetGDBRemote().SendContinuePacketAndWaitForResponse (process, continue_cstr, continue_cstr_len, response);
+ }
+ // If in All-Stop-Mode
+ else
+ {
+ StateType stop_state = process->GetGDBRemote().SendContinuePacketAndWaitForResponse (process, continue_cstr, continue_cstr_len, response);
- // We need to immediately clear the thread ID list so we are sure to get a valid list of threads.
- // The thread ID list might be contained within the "response", or the stop reply packet that
- // caused the stop. So clear it now before we give the stop reply packet to the process
- // using the process->SetLastStopPacket()...
- process->ClearThreadIDList ();
+ // We need to immediately clear the thread ID list so we are sure to get a valid list of threads.
+ // The thread ID list might be contained within the "response", or the stop reply packet that
+ // caused the stop. So clear it now before we give the stop reply packet to the process
+ // using the process->SetLastStopPacket()...
+ process->ClearThreadIDList ();
- switch (stop_state)
- {
- case eStateStopped:
- case eStateCrashed:
- case eStateSuspended:
- process->SetLastStopPacket (response);
- process->SetPrivateState (stop_state);
- break;
+ switch (stop_state)
+ {
+ case eStateStopped:
+ case eStateCrashed:
+ case eStateSuspended:
+ process->SetLastStopPacket (response);
+ process->SetPrivateState (stop_state);
+ break;
- case eStateExited:
+ case eStateExited:
+ {
+ process->SetLastStopPacket (response);
+ process->ClearThreadIDList();
+ response.SetFilePos(1);
+
+ int exit_status = response.GetHexU8();
+ const char *desc_cstr = NULL;
+ StringExtractor extractor;
+ std::string desc_string;
+ if (response.GetBytesLeft() > 0 && response.GetChar('-') == ';')
{
- process->SetLastStopPacket (response);
- process->ClearThreadIDList();
- response.SetFilePos(1);
-
- int exit_status = response.GetHexU8();
- const char *desc_cstr = NULL;
- StringExtractor extractor;
- std::string desc_string;
- if (response.GetBytesLeft() > 0 && response.GetChar('-') == ';')
+ std::string desc_token;
+ while (response.GetNameColonValue (desc_token, desc_string))
{
- std::string desc_token;
- while (response.GetNameColonValue (desc_token, desc_string))
+ if (desc_token == "description")
{
- if (desc_token == "description")
- {
- extractor.GetStringRef().swap(desc_string);
- extractor.SetFilePos(0);
- extractor.GetHexByteString (desc_string);
- desc_cstr = desc_string.c_str();
- }
+ extractor.GetStringRef().swap(desc_string);
+ extractor.SetFilePos(0);
+ extractor.GetHexByteString (desc_string);
+ desc_cstr = desc_string.c_str();
}
}
- process->SetExitStatus(exit_status, desc_cstr);
- done = true;
- break;
}
- case eStateInvalid:
+ process->SetExitStatus(exit_status, desc_cstr);
+ done = true;
+ break;
+ }
+ case eStateInvalid:
+ {
+ // Check to see if we were trying to attach and if we got back
+ // the "E87" error code from debugserver -- this indicates that
+ // the process is not debuggable. Return a slightly more helpful
+ // error message about why the attach failed.
+ if (::strstr (continue_cstr, "vAttach") != NULL
+ && response.GetError() == 0x87)
{
- // Check to see if we were trying to attach and if we got back
- // the "E87" error code from debugserver -- this indicates that
- // the process is not debuggable. Return a slightly more helpful
- // error message about why the attach failed.
- if (::strstr (continue_cstr, "vAttach") != NULL
- && response.GetError() == 0x87)
- {
- process->SetExitStatus(-1, "cannot attach to process due to System Integrity Protection");
- }
- // E01 code from vAttach means that the attach failed
- if (::strstr (continue_cstr, "vAttach") != NULL
- && response.GetError() == 0x1)
- {
- process->SetExitStatus(-1, "unable to attach");
- }
- else
- {
- process->SetExitStatus(-1, "lost connection");
- }
- break;
+ process->SetExitStatus(-1, "cannot attach to process due to System Integrity Protection");
+ }
+ // E01 code from vAttach means that the attach failed
+ if (::strstr (continue_cstr, "vAttach") != NULL
+ && response.GetError() == 0x1)
+ {
+ process->SetExitStatus(-1, "unable to attach");
+ }
+ else
+ {
+ process->SetExitStatus(-1, "lost connection");
}
-
- default:
- process->SetPrivateState (stop_state);
break;
- } // switch(stop_state)
- } // else // if in All-stop-mode
- } // if (continue_packet)
- } // case eBroadcastBitAysncContinue
- break;
+ }
- case eBroadcastBitAsyncThreadShouldExit:
- if (log)
- log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") got eBroadcastBitAsyncThreadShouldExit...", __FUNCTION__, arg, process->GetID());
- done = true;
- break;
+ default:
+ process->SetPrivateState (stop_state);
+ break;
+ } // switch(stop_state)
+ } // else // if in All-stop-mode
+ } // if (continue_packet)
+ } // case eBroadcastBitAysncContinue
+ break;
- default:
- if (log)
- log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") got unknown event 0x%8.8x", __FUNCTION__, arg, process->GetID(), event_type);
- done = true;
- break;
- }
+ case eBroadcastBitAsyncThreadShouldExit:
+ if (log)
+ log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") got eBroadcastBitAsyncThreadShouldExit...", __FUNCTION__, arg, process->GetID());
+ done = true;
+ break;
+
+ default:
+ if (log)
+ log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") got unknown event 0x%8.8x", __FUNCTION__, arg, process->GetID(), event_type);
+ done = true;
+ break;
}
- else if (event_sp->BroadcasterIs (&process->m_gdb_comm))
+ }
+ else if (event_sp->BroadcasterIs (&process->m_gdb_comm))
+ {
+ switch (event_type)
{
- switch (event_type)
- {
- case Communication::eBroadcastBitReadThreadDidExit:
- process->SetExitStatus (-1, "lost connection");
- done = true;
- break;
-
- case GDBRemoteCommunication::eBroadcastBitGdbReadThreadGotNotify:
- {
- lldb_private::Event *event = event_sp.get();
- const EventDataBytes *continue_packet = EventDataBytes::GetEventDataFromEvent(event);
- StringExtractorGDBRemote notify((const char*)continue_packet->GetBytes());
- // Hand this over to the process to handle
- process->HandleNotifyPacket(notify);
- break;
- }
+ case Communication::eBroadcastBitReadThreadDidExit:
+ process->SetExitStatus (-1, "lost connection");
+ done = true;
+ break;
- default:
- if (log)
- log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") got unknown event 0x%8.8x", __FUNCTION__, arg, process->GetID(), event_type);
- done = true;
- break;
+ case GDBRemoteCommunication::eBroadcastBitGdbReadThreadGotNotify:
+ {
+ lldb_private::Event *event = event_sp.get();
+ const EventDataBytes *continue_packet = EventDataBytes::GetEventDataFromEvent(event);
+ StringExtractorGDBRemote notify((const char*)continue_packet->GetBytes());
+ // Hand this over to the process to handle
+ process->HandleNotifyPacket(notify);
+ break;
}
+
+ default:
+ if (log)
+ log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") got unknown event 0x%8.8x", __FUNCTION__, arg, process->GetID(), event_type);
+ done = true;
+ break;
}
}
- else
- {
- if (log)
- log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") listener.WaitForEvent (NULL, event_sp) => false", __FUNCTION__, arg, process->GetID());
- done = true;
- }
+ }
+ else
+ {
+ if (log)
+ log->Printf ("ProcessGDBRemote::%s (arg = %p, pid = %" PRIu64 ") listener.WaitForEvent (NULL, event_sp) => false", __FUNCTION__, arg, process->GetID());
+ done = true;
}
}
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=246756&r1=246755&r2=246756&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Thu Sep 3 04:36:22 2015
@@ -358,6 +358,7 @@ protected:
Mutex m_last_stop_packet_mutex;
GDBRemoteDynamicRegisterInfo m_register_info;
Broadcaster m_async_broadcaster;
+ Listener m_async_listener;
HostThread m_async_thread;
Mutex m_async_thread_state_mutex;
typedef std::vector<lldb::tid_t> tid_collection;
Modified: lldb/trunk/test/dosep.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/dosep.py?rev=246756&r1=246755&r2=246756&view=diff
==============================================================================
--- lldb/trunk/test/dosep.py (original)
+++ lldb/trunk/test/dosep.py Thu Sep 3 04:36:22 2015
@@ -291,7 +291,6 @@ def getExpectedTimeouts(platform_name):
if target.startswith("linux"):
expected_timeout |= {
"TestAttachDenied.py",
- "TestAttachResume.py",
"TestProcessAttach.py",
"TestConnectRemote.py",
"TestCreateAfterAttach.py",
More information about the lldb-commits
mailing list