Rewrite gdb-remote's SendContinuePacketAndWaitForResponse

[labath]

Summary

SendContinuePacketAndWaitForResponse was huge function with very complex interactions with
several other functions (SendAsyncSignal, SendInterrupt, SendPacket). This meant that making any
changes to how packet sending functions and threads interact was very difficult and error-prone.

This change does not add any functionality yet, it merely paves the way for future changes. In a
follow-up, I plan to add the ability to have multiple query packets in flight (i.e.,
request,request,response,response instead of the usual request,response sequences) and use that
to speed up qModuleInfo packet processing.

Here, I introduce two special kinds of locks: ContinueLock, which is used by the continue thread,
and Lock, which is used by everyone else. ContinueLock (atomically) sends a continue packet, and
blocks any other async threads from accessing the connection. Other threads create an instance of
the Lock object when they want to access the connection. This object, while in scope prevents the
continue from being send. Optionally, it can also interrupt the process to gain access to the
connection for async processing.

Most of the syncrhonization logic is encapsulated within these two classes. Some of it still
had to bleed over into the SendContinuePacketAndWaitForResponse, but the function is still much
more manageable than before -- partly because of most of the work is done in the ContinueLock
class, and partly because I have factored out a lot of the packet processing code separate
functions (this also makes the functionality more easily testable). Most importantly, there is
none of syncrhonization code in the async thread users -- as far as they are concerned, they just
need to declare a Lock object, and they are good to go (SendPacketAndWaitForResponse is now a
very thin wrapper around the NoLock version of the function, whereas previously it had over 100
lines of synchronization code).  This will make my follow up changes there easy.

I have written a number of unit tests for the new code and I have ran the test suite on linux and
osx with no regressions.

[labath]

clean up a bit

[labath]

Move things into place

[labath]

Use StringRef in the interfaces, where possible. Unify packet/payload naming in interfaces.

[labath]

Fix a race in m_should_stop handlind + drive-by refactor

[labath]

Previous change fixed the race, but made it uncondtitionally broken. Fix that.

[tberghammer]

source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp

57

(nit): I would make it an assert as it is clearly an LLDB bug if we hit this code path

82

(nit): Not needed

292

Should we do some sort of cleanup here (e.g. m_async_count--)?

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

23

(nit): I think you don't need this

unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp

34

(nit): Please put these into an anonymous namepsace

[labath]

Address review comments. Thanks.

[labath]

final tweaks before sending out for review

[clayborg]

This provides a clearer implementation of our async code that grew over the years and was bolted on, thanks for digging into this.

Looks good a few changes, see inlined comments.

I have some reservations about your future changes to add concurrent packets. We should discuss this at length before you try to make any changes on the mailing list.

source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp

58

if you add an assert, please use lldbassert

149–151

Do we need to call lock.DidInterrupt() twice?

262–269

We can probably switch over using a new function that was added after this code was created:

size_t
StringExtractor::GetHexByteString (std::string &str);

So this could be:

inferior_stdout.reserve(packet.GetBytesLeft() / 2);
packet. GetHexByteString(inferior_stdout.reserve);

source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h

29–30

Why not just use "const std::string &" here instead of llvm::StringRef? It is OK to use std::string in internal APIs and llvm::StringRef buys us nothing.

32

Why not just use "const std::string &" here instead of llvm::StringRef? It is OK to use std::string in internal APIs and llvm::StringRef buys us nothing.

[labath]

Address review comments.

[labath]

In D22629#498135, @clayborg wrote:

I have some reservations about your future changes to add concurrent packets. We should discuss this at length before you try to make any changes on the mailing list.

Agreed. I have a follow-up almost ready. I think I'll be able to upload it as a WIP later in the day. I think it will be easier to explain the idea with a concrete example.

source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp

58

This comment shows up in the wrong spot, I am moving it to the place where the assert() is

149–151

The call is cheap so I don't think it matters, but I've reshuffled it a bit and now it's one.

298

The assert() (and two more in the lock() function). I am going to change it back to lldbassert, but this checks a very critical part of the internal locking logic, and I would very much doubt our ability recover if these conditions are not met.

source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h

32

In the previous case it does not matter (right now (*)), but it this case it actually saves one string copy: The caller does a substr(1) on the received packet (to remove the leading 'A'), which is a cheap operation on a StringRef, but not on a string.

(*) It does however, future-proof the api. If the caller or the callee end up using something other than a std::string in the future, the API will remain the same, and cheap, as probably StringRef can be used to convert to/from that.

In general, I would reverse the logic here: there is nothing that can be done with a string that you cannot do with a StringRef(**), so why use the former? It's also consistent with the llvm guidelines.

(**) The only exception here is the c_str() function, but if you use StringRefs everywhere then you don't need to call it. Also, mitigating circumstances are: - a lot of our apis already use const char*+length style, which is just a hand-rolled StringRef and is cheap to convert back and forth; by the time you end up calling c_str(), you have probably have already saved a couple of copies, so doing one copy there is not that bad.

[clayborg]

I am fine with the llvm::StringRef until it causes a crash. So really look at all the ways you construct a llvm::StringRef and make sure it won't assert ever if you leave it as a llvm::StringRef.

source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp

269

Actually, feel free to switch over to using GetHexByteString, but maybe just put the reserve part into that function's implementation? Then this code would be just:

packet. GetHexByteString(inferior_stdout);

source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h

31

I just really don't like that a llvm::StringRef constructed with a null "const char *" will crash LLDB due to an assertion. It has cause crashes in the past when you have the result of a function being fed into a llvm::StringRef. I really want to make a lldb::StringRef that subclasses llvm::StringRef and fixes this as I really don't think it is useful. Why can't an llvm::StringRef be constructed with NULL????

[labath]

source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp

269

Will do. At that point this whole function becomes pretty irrelevant and i will inline it back into the caller.

source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h

31

I understand your concerns, but i also see the other side's point, who consider a null pointer an invalid input and the responsibility of the user to deal with that.

That said, we as a user have a lot of places which treat null as an empty string, so we need to do something about that. Instead of a new (sub-)class, I'd go for making a factory function instead (lldb::makeStringRef()?) and say we use that whereever we are not sure about the nullness of the pointer. The advantage of that being you don't have to worry about type conversions when dealing with different StringRefs (and once our StringRef is constructed, it works just like the vanilla object). In time, the need for that function should even diminish, as once we are already using a StringRef, we don't have to worry about the whole null pointer business.

BTW, constructing a std::string from a null pointer is also undefined behaviour, and implementations I've seen either assert or crash when trying do to a strlen().

source/Plugins/Process/gdb-remote/CMakeLists.txt

 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   include_directories(${LIBXML2_INCLUDE_DIR})
 endif()
 
 add_lldb_library(lldbPluginProcessGDBRemote
+  GDBRemoteClientBase.cpp
   GDBRemoteCommunication.cpp
   GDBRemoteCommunicationClient.cpp
   GDBRemoteCommunicationServer.cpp
   GDBRemoteCommunicationServerCommon.cpp
   GDBRemoteCommunicationServerLLGS.cpp
   GDBRemoteCommunicationServerPlatform.cpp
   GDBRemoteRegisterContext.cpp
   ProcessGDBRemote.cpp
   ProcessGDBRemoteLog.cpp
   ThreadGDBRemote.cpp
   )

source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h

+//===-- GDBRemoteClientBase.h -----------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_GDBRemoteClientBase_h_
+#define liblldb_GDBRemoteClientBase_h_
+
+#include "GDBRemoteCommunication.h"
+
+#include <condition_variable>
+
+namespace lldb_private
+{
+namespace process_gdb_remote
+{
+
+class GDBRemoteClientBase : public GDBRemoteCommunication
+{
+public:
+    struct ContinueDelegate
+    {
+        virtual ~ContinueDelegate();
+        virtual void
+        HandleAsyncStdout(llvm::StringRef out) = 0;
+        virtual void
+        HandleAsyncMisc(llvm::StringRef data) = 0;
+        virtual void
+        HandleStopReply() = 0;
+    };
+
+    GDBRemoteClientBase(const char *comm_name, const char *listener_name);
+
+    bool
+    SendAsyncSignal(int signo);
+
+    bool
+    Interrupt();
+
+    lldb::StateType
+    SendContinuePacketAndWaitForResponse(ContinueDelegate &delegate, const UnixSignals &signals,
+                                         llvm::StringRef payload, StringExtractorGDBRemote &response);
+
+    PacketResult
+    SendPacketAndWaitForResponse(const char *payload, size_t len, StringExtractorGDBRemote &response, bool send_async)
+    {
+        return SendPacketAndWaitForResponse(llvm::StringRef(payload, len), response, send_async);
+    }
+
+    PacketResult
+    SendPacketAndWaitForResponse(llvm::StringRef payload, StringExtractorGDBRemote &response, bool send_async);
+
+    bool
+    SendvContPacket(llvm::StringRef payload, StringExtractorGDBRemote &response);
+
+    class Lock
+    {
+    public:
+        Lock(GDBRemoteClientBase &comm, bool interrupt);
+        ~Lock();
+
+        explicit operator bool() { return m_acquired; }
+
+        // Whether we had to interrupt the continue thread to acquire the connection.
+        bool
+        DidInterrupt() const
+        {
+            return m_did_interrupt;
+        }
+
+    private:
+        std::unique_lock<std::recursive_mutex> m_async_lock;
+        GDBRemoteClientBase &m_comm;
+        bool m_acquired;
+        bool m_did_interrupt;
+
+        void
+        SyncWithContinueThread(bool interrupt);
+    };
+
+protected:
+    void
+    HandleAsyncStdout(ContinueDelegate &delegate, StringExtractorGDBRemote &payload);
+
+    PacketResult
+    SendPacketAndWaitForResponseNoLock(llvm::StringRef payload, StringExtractorGDBRemote &response);
+
+    virtual void
+    OnRunPacketSent(bool first);
+
+private:
+    // Variables handling synchronization between the Continue thread and any other threads
+    // wishing to send packets over the connection. Either the continue thread has control over
+    // the connection (m_is_running == true) or the connection is free for an arbitrary number of
+    // other senders to take which indicate their interest by incrementing m_async_count.
+    // Semantics of individual states:
+    // - m_continue_packet == false, m_async_count == 0: connection is free
+    // - m_continue_packet == true, m_async_count == 0: only continue thread is present
+    // - m_continue_packet == true, m_async_count > 0: continue thread has control, async threads
+    //   should interrupt it and wait for it to set m_continue_packet to false
+    // - m_continue_packet == false, m_async_count > 0: async threads have control, continue
+    //   thread needs to wait for them to finish (m_async_count goes down to 0).
+    std::mutex m_mutex;
+    std::condition_variable m_cv;
+    // Packet with which to resume after an async interrupt. Can be changed by an async thread
+    // e.g. to inject a signal.
+    std::string m_continue_packet;
+    // When was the interrupt packet sent. Used to make sure we time out if the stub does not
+    // respond to interrupt requests.
+    std::chrono::time_point<std::chrono::steady_clock> m_interrupt_time;
+    uint32_t m_async_count;
+    bool m_is_running;
+    bool m_should_stop; // Whether we should resume after a stop.
+    // end of continue thread synchronization block
+
+    // This handles the synchronization between individual async threads. For now they just use a
+    // simple mutex.
+    std::recursive_mutex m_async_mutex;
+
+    bool
+    ShouldStop(const UnixSignals &signals, StringExtractorGDBRemote &response);
+
+    class ContinueLock
+    {
+    public:
+        enum class LockResult
+        {
+            Success,
+            Cancelled,
+            Failed
+        };
+
+        explicit ContinueLock(GDBRemoteClientBase &comm);
+        ~ContinueLock();
+        explicit operator bool() { return m_acquired; }
+
+        LockResult
+        lock();
+
+        void
+        unlock();
+
+    private:
+        GDBRemoteClientBase &m_comm;
+        bool m_acquired;
+    };
+};
+
+} // namespace process_gdb_remote
+} // namespace lldb_private
+
+#endif // liblldb_GDBRemoteCommunicationClient_h_

source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp

+//===-- GDBRemoteClientBase.cpp ---------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "GDBRemoteClientBase.h"
+
+#include "llvm/ADT/StringExtras.h"
+
+#include "lldb/Target/UnixSignals.h"
+#include "lldb/Utility/LLDBAssert.h"
+
+#include "ProcessGDBRemoteLog.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::process_gdb_remote;
+
+static const std::chrono::seconds kInterruptTimeout(5);
+
+/////////////////////////
+// GDBRemoteClientBase //
+/////////////////////////
+
+GDBRemoteClientBase::ContinueDelegate::~ContinueDelegate() = default;
+
+GDBRemoteClientBase::GDBRemoteClientBase(const char *comm_name, const char *listener_name)
+    : GDBRemoteCommunication(comm_name, listener_name), m_async_count(0), m_is_running(false), m_should_stop(false)
+{
+}
+
+StateType
+GDBRemoteClientBase::SendContinuePacketAndWaitForResponse(ContinueDelegate &delegate, const UnixSignals &signals,
+                                                          llvm::StringRef payload, StringExtractorGDBRemote &response)
+{
+    Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+    response.Clear();
+
+    m_continue_packet = payload;
+    ContinueLock cont_lock(*this);
+    if (!cont_lock)
+        return eStateInvalid;
+    OnRunPacketSent(true);
+
+    for (;;)
+    {
+        PacketResult read_result = ReadPacket(
+            response, std::chrono::duration_cast<std::chrono::microseconds>(kInterruptTimeout).count(), false);
+        switch (read_result)
+        {
+            case PacketResult::ErrorReplyTimeout:
+            {
+                std::lock_guard<std::mutex> lock(m_mutex);
+                if (m_async_count == 0)
+                    continue;
+                if (std::chrono::steady_clock::now() >= m_interrupt_time + kInterruptTimeout)
+                    return eStateInvalid;
+            }
+            case PacketResult::Success:
+                break;
+            default:
+                if (log)
+                    log->Printf("GDBRemoteClientBase::%s () ReadPacket(...) => false", __FUNCTION__);
+                return eStateInvalid;
+        }
+        if (response.Empty())
+            return eStateInvalid;
+
+        const char stop_type = response.GetChar();
+        if (log)
+            log->Printf("GDBRemoteClientBase::%s () got packet: %s", __FUNCTION__, response.GetStringRef().c_str());
+
+        switch (stop_type)
+        {
+            case 'W':
+            case 'X':
+                return eStateExited;
+            case 'E':
+                // ERROR
+                return eStateInvalid;
+            default:
+                if (log)
+                    log->Printf("GDBRemoteClientBase::%s () unrecognized async packet", __FUNCTION__);
+                return eStateInvalid;
+            case 'O':
+                HandleAsyncStdout(delegate, response);
+                break;
+            case 'A':
+                delegate.HandleAsyncMisc(llvm::StringRef(response.GetStringRef()).substr(1));
+                break;
+            case 'T':
+            case 'S':
+                // Do this with the continue lock held.
+                const bool should_stop = ShouldStop(signals, response);
+                response.SetFilePos(0);
+
+                // The packet we should resume with. In the future
+                // we should check our thread list and "do the right thing"
+                // for new threads that show up while we stop and run async
+                // packets. Setting the packet to 'c' to continue all threads
+                // is the right thing to do 99.99% of the time because if a
+                // thread was single stepping, and we sent an interrupt, we
+                // will notice above that we didn't stop due to an interrupt
+                // but stopped due to stepping and we would _not_ continue.
+                // This packet may get modified by the async actions (e.g. to send a signal).
+                m_continue_packet = 'c';
+                cont_lock.unlock();
+
+                delegate.HandleStopReply();
+                if (should_stop)
+                    return eStateStopped;
+
+                switch (cont_lock.lock())
+                {
+                    case ContinueLock::LockResult::Success:
+                        break;
+                    case ContinueLock::LockResult::Failed:
+                        return eStateInvalid;
+                    case ContinueLock::LockResult::Cancelled:
+                        return eStateStopped;
+                }
+                OnRunPacketSent(false);
+                break;
+        }
+    }
+}
+
+bool
+GDBRemoteClientBase::SendAsyncSignal(int signo)
+{
+    Lock lock(*this, true);
+    if (!lock || !lock.DidInterrupt())
+        return false;
+
+    m_continue_packet = 'C';
+    m_continue_packet += llvm::hexdigit((signo / 16) % 16);
+    m_continue_packet += llvm::hexdigit(signo % 16);
+    return true;
+}
+
+bool
+GDBRemoteClientBase::Interrupt()
+{
+    Lock lock(*this, true);
+    if (!lock.DidInterrupt())
+        return false;
+    m_should_stop = true;
+    return true;
+}
+GDBRemoteCommunication::PacketResult
+GDBRemoteClientBase::SendPacketAndWaitForResponse(llvm::StringRef payload, StringExtractorGDBRemote &response,
+                                                  bool send_async)
+{
+    Lock lock(*this, send_async);
+    if (!lock)
+    {
+        if (Log *log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS))
+            log->Printf("GDBRemoteClientBase::%s failed to get mutex, not sending packet '%.*s' (send_async=%d)",
+                        __FUNCTION__, int(payload.size()), payload.data(), send_async);
+        return PacketResult::ErrorSendFailed;
+    }
+
+    return SendPacketAndWaitForResponseNoLock(payload, response);
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(llvm::StringRef payload, StringExtractorGDBRemote &response)
+{
+    PacketResult packet_result = SendPacketNoLock(payload.data(), payload.size());
+    if (packet_result != PacketResult::Success)
+        return packet_result;
+
+    const size_t max_response_retries = 3;
+    for (size_t i = 0; i < max_response_retries; ++i)
+    {
+        packet_result = ReadPacket(response, GetPacketTimeoutInMicroSeconds(), true);
+        // Make sure we received a response
+        if (packet_result != PacketResult::Success)
+            return packet_result;
+        // Make sure our response is valid for the payload that was sent
+        if (response.ValidateResponse())
+            return packet_result;
+        // Response says it wasn't valid
+        Log *log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS);
+        if (log)
+            log->Printf("error: packet with payload \"%.*s\" got invalid response \"%s\": %s", int(payload.size()),
+                        payload.data(), response.GetStringRef().c_str(),
+                        (i == (max_response_retries - 1)) ? "using invalid response and giving up"
+                                                          : "ignoring response and waiting for another");
+    }
+    return packet_result;
+}
+
+bool
+GDBRemoteClientBase::SendvContPacket(llvm::StringRef payload, StringExtractorGDBRemote &response)
+{
+    Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+    if (log)
+        log->Printf("GDBRemoteCommunicationClient::%s ()", __FUNCTION__);
+
+    // we want to lock down packet sending while we continue
+    Lock lock(*this, true);
+
+    if (log)
+        log->Printf("GDBRemoteCommunicationClient::%s () sending vCont packet: %.*s", __FUNCTION__, int(payload.size()),
+                    payload.data());
+
+    if (SendPacketNoLock(payload.data(), payload.size()) != PacketResult::Success)
+        return false;
+
+    OnRunPacketSent(true);
+
+    // wait for the response to the vCont
+    if (ReadPacket(response, UINT32_MAX, false) == PacketResult::Success)
+    {
+        if (response.IsOKResponse())
+            return true;
+    }
+
+    return false;
+}
+bool
+GDBRemoteClientBase::ShouldStop(const UnixSignals &signals, StringExtractorGDBRemote &response)
+{
+    std::lock_guard<std::mutex> lock(m_mutex);
+
+    if (m_async_count == 0)
+        return true; // We were not interrupted. The process stopped on its own.
+
+    // Older debugserver stubs (before April 2016) can return two
+    // stop-reply packets in response to a ^C packet.
+    // Additionally, all debugservers still return two stop replies if
+    // the inferior stops due to some other reason before the remote
+    // stub manages to interrupt it. We need to wait for this
+    // additional packet to make sure the packet sequence does not get
+    // skewed.
+    StringExtractorGDBRemote extra_stop_reply_packet;
+    uint32_t timeout_usec = 100000; // 100ms
+    ReadPacket(extra_stop_reply_packet, timeout_usec, false);
+
+    // Interrupting is typically done using SIGSTOP or SIGINT, so if
+    // the process stops with some other signal, we definitely want to
+    // stop.
+    const uint8_t signo = response.GetHexU8(UINT8_MAX);
+    if (signo != signals.GetSignalNumberFromName("SIGSTOP") && signo != signals.GetSignalNumberFromName("SIGINT"))
+        return true;
+
+    // We probably only stopped to perform some async processing, so continue after that is done.
+    // TODO: This is not 100% correct, as the process may have been stopped with SIGINT or SIGSTOP
+    // that was not caused by us (e.g. raise(SIGINT)). This will normally cause a stop, but if it's
+    // done concurrently with a async interrupt, that stop will get eaten (llvm.org/pr20231).
+    return false;
+}
+
+void
+GDBRemoteClientBase::HandleAsyncStdout(ContinueDelegate &delegate, StringExtractorGDBRemote &packet)
+{
+    std::string inferior_stdout;
+    inferior_stdout.reserve(packet.GetBytesLeft() / 2);
+    packet.GetHexByteString(inferior_stdout);
+    delegate.HandleAsyncStdout(inferior_stdout);
+}
+
+void
+GDBRemoteClientBase::OnRunPacketSent(bool first)
+{
+    if (first)
+        BroadcastEvent(eBroadcastBitRunPacketSent, NULL);
+}
+
+///////////////////////////////////////
+// GDBRemoteClientBase::ContinueLock //
+///////////////////////////////////////
+
+GDBRemoteClientBase::ContinueLock::ContinueLock(GDBRemoteClientBase &comm) : m_comm(comm), m_acquired(false)
+{
+    lock();
+}
+
+GDBRemoteClientBase::ContinueLock::~ContinueLock()
+{
+    if (m_acquired)
+        unlock();
+}
+
+void
+GDBRemoteClientBase::ContinueLock::unlock()
+{
+    lldbassert(m_acquired);
+    {
+        std::unique_lock<std::mutex> lock(m_comm.m_mutex);
+        m_comm.m_is_running = false;
+    }
+    m_comm.m_cv.notify_all();
+    m_acquired = false;
+}
+
+GDBRemoteClientBase::ContinueLock::LockResult
+GDBRemoteClientBase::ContinueLock::lock()
+{
+    Log *log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS);
+    if (log)
+        log->Printf("GDBRemoteClientBase::ContinueLock::%s() resuming with %s", __FUNCTION__,
+                    m_comm.m_continue_packet.c_str());
+
+    lldbassert(!m_acquired);
+    std::unique_lock<std::mutex> lock(m_comm.m_mutex);
+    m_comm.m_cv.wait(lock, [this] { return m_comm.m_async_count == 0; });
+    if (m_comm.m_should_stop)
+    {
+        m_comm.m_should_stop = false;
+        return LockResult::Cancelled;
+    }
+    if (m_comm.SendPacketNoLock(m_comm.m_continue_packet.data(), m_comm.m_continue_packet.size()) !=
+        PacketResult::Success)
+        return LockResult::Failed;
+
+    lldbassert(!m_comm.m_is_running);
+    m_comm.m_is_running = true;
+    m_acquired = true;
+    return LockResult::Success;
+}
+
+///////////////////////////////
+// GDBRemoteClientBase::Lock //
+///////////////////////////////
+
+GDBRemoteClientBase::Lock::Lock(GDBRemoteClientBase &comm, bool interrupt)
+    : m_async_lock(comm.m_async_mutex, std::defer_lock), m_comm(comm), m_acquired(false), m_did_interrupt(false)
+{
+    SyncWithContinueThread(interrupt);
+    if (m_acquired)
+        m_async_lock.lock();
+}
+
+void
+GDBRemoteClientBase::Lock::SyncWithContinueThread(bool interrupt)
+{
+    Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+    std::unique_lock<std::mutex> lock(m_comm.m_mutex);
+    if (m_comm.m_is_running && !interrupt)
+        return; // We were asked to avoid interrupting the sender. Lock is not acquired.
+
+    ++m_comm.m_async_count;
+    if (m_comm.m_is_running)
+    {
+        if (m_comm.m_async_count == 1)
+        {
+            // The sender has sent the continue packet and we are the first async packet. Let's interrupt it.
+            const char ctrl_c = '\x03';
+            ConnectionStatus status = eConnectionStatusSuccess;
+            size_t bytes_written = m_comm.Write(&ctrl_c, 1, status, NULL);
+            if (bytes_written == 0)
+            {
+                --m_comm.m_async_count;
+                if (log)
+                    log->Printf("GDBRemoteClientBase::Lock::Lock failed to send interrupt packet");
+                return;
+            }
+            if (log)
+                log->PutCString("GDBRemoteClientBase::Lock::Lock sent packet: \\x03");
+            m_comm.m_interrupt_time = std::chrono::steady_clock::now();
+        }
+        m_comm.m_cv.wait(lock, [this] { return m_comm.m_is_running == false; });
+        m_did_interrupt = true;
+    }
+    m_acquired = true;
+}
+
+GDBRemoteClientBase::Lock::~Lock()
+{
+    if (!m_acquired)
+        return;
+    {
+        std::unique_lock<std::mutex> lock(m_comm.m_mutex);
+        --m_comm.m_async_count;
+    }
+    m_comm.m_cv.notify_one();
+}

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h

public:
 
     size_t
     SendNack ();
 
     char
     CalculcateChecksum (const char *payload,
                         size_t payload_length);
 
-    bool
-    GetSequenceMutex(Mutex::Locker& locker, const char *failure_message = nullptr);
-
     PacketType
     CheckForPacket (const uint8_t *src, 
                     size_t src_len, 
                     StringExtractorGDBRemote &packet);
 
     bool
-    IsRunning() const
-    {
-        return m_public_is_running.GetValue();
-    }
-
-    bool
     GetSendAcks ()
     {
         return m_send_acks;
     }
 
     //------------------------------------------------------------------
     // Client and server must implement these pure virtual functions
     //------------------------------------------------------------------
protected:
         uint32_t m_curr_idx;
         uint32_t m_total_packet_count;
         mutable bool m_dumped_to_log;
     };
 
     uint32_t m_packet_timeout;
     uint32_t m_echo_number;
     LazyBool m_supports_qEcho;
-#ifdef ENABLE_MUTEX_ERROR_CHECKING
-    TrackingMutex m_sequence_mutex;
-#else
-    Mutex m_sequence_mutex;    // Restrict access to sending/receiving packets to a single thread at a time
-#endif
-    Predicate<bool> m_public_is_running;
-    Predicate<bool> m_private_is_running;
     History m_history;
     bool m_send_acks;
     bool m_is_platform; // Set to true if this class represents a platform,
                         // false if this class represents a debug session for
                         // a single process
     
     CompressionType m_compression_type;
 
     PacketResult
-    SendPacket (const char *payload,
-                size_t payload_length);
-
-    PacketResult
     SendPacketNoLock (const char *payload, 
                       size_t payload_length);
 
     PacketResult
     ReadPacket (StringExtractorGDBRemote &response, uint32_t timeout_usec, bool sync_on_timeout);
 
     // Pop a packet from the queue in a thread safe manner
     PacketResult
     PopPacketFromQueue (StringExtractorGDBRemote &response, uint32_t timeout_usec);
 
     PacketResult
     WaitForPacketWithTimeoutMicroSecondsNoLock (StringExtractorGDBRemote &response, 
                                                 uint32_t timeout_usec,
                                                 bool sync_on_timeout);
 
     bool
-    WaitForNotRunningPrivate (const TimeValue *timeout_ptr);
-
-    bool
     CompressionIsEnabled ()
     {
         return m_compression_type != CompressionType::None;
     }
 
     // If compression is enabled, decompress the packet in m_bytes and update
     // m_bytes with the uncompressed version.
     // Returns 'true' packet was decompressed and m_bytes is the now-decompressed text.

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

GDBRemoteCommunication::GDBRemoteCommunication(const char *comm_name,
     Communication(comm_name),
 #ifdef LLDB_CONFIGURATION_DEBUG
     m_packet_timeout (1000),
 #else
     m_packet_timeout (1),
 #endif
     m_echo_number(0),
     m_supports_qEcho (eLazyBoolCalculate),
-    m_sequence_mutex (Mutex::eMutexTypeRecursive),
-    m_public_is_running (false),
-    m_private_is_running (false),
     m_history (512),
     m_send_acks (true),
     m_compression_type (CompressionType::None),
     m_listen_url ()
 {
 }
 
 //----------------------------------------------------------------------
GDBRemoteCommunication::SendNack ()
     const size_t bytes_written = Write (&ch, 1, status, NULL);
     if (log)
         log->Printf("<%4" PRIu64 "> send packet: %c", (uint64_t)bytes_written, ch);
     m_history.AddPacket (ch, History::ePacketTypeSend, bytes_written);
     return bytes_written;
 }
 
 GDBRemoteCommunication::PacketResult
-GDBRemoteCommunication::SendPacket (const char *payload, size_t payload_length)
-{
-    Mutex::Locker locker(m_sequence_mutex);
-    return SendPacketNoLock (payload, payload_length);
-}
-
-GDBRemoteCommunication::PacketResult
 GDBRemoteCommunication::SendPacketNoLock (const char *payload, size_t payload_length)
 {
     if (IsConnected())
     {
         StreamString packet(0, 4, eByteOrderBig);
 
         packet.PutChar('$');
         packet.Write (payload, payload_length);
if (result == PacketResult::Success)
         if (packet.GetResponseType() == StringExtractorGDBRemote::ResponseType::eAck)
             return PacketResult::Success;
         else
             return PacketResult::ErrorSendAck;
     }
     return result;
 }
 
-bool
-GDBRemoteCommunication::GetSequenceMutex (Mutex::Locker& locker, const char *failure_message)
-{
-    if (IsRunning())
-        return locker.TryLock (m_sequence_mutex, failure_message);
-
-    locker.Lock (m_sequence_mutex);
-    return true;
-}
-
-
-bool
-GDBRemoteCommunication::WaitForNotRunningPrivate (const TimeValue *timeout_ptr)
-{
-    return m_private_is_running.WaitForValueEqualTo (false, timeout_ptr, NULL);
-}
-
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunication::ReadPacket (StringExtractorGDBRemote &response, uint32_t timeout_usec, bool sync_on_timeout)
 {
    if (m_read_thread_enabled)
        return PopPacketFromQueue (response, timeout_usec);
    else
        return WaitForPacketWithTimeoutMicroSecondsNoLock (response, timeout_usec, sync_on_timeout);
 }

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

 //===-- GDBRemoteCommunicationClient.h --------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_GDBRemoteCommunicationClient_h_
 #define liblldb_GDBRemoteCommunicationClient_h_
 
 // C Includes
 // C++ Includes
+#include <chrono>
 #include <map>
 #include <mutex>
 #include <string>
 #include <vector>
 
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/StructuredData.h"
 #include "lldb/Target/Process.h"
 
-#include "GDBRemoteCommunication.h"
+#include "GDBRemoteClientBase.h"
 
 namespace lldb_private {
 namespace process_gdb_remote {
 
-class GDBRemoteCommunicationClient : public GDBRemoteCommunication
+class GDBRemoteCommunicationClient : public GDBRemoteClientBase
 {
 public:
     GDBRemoteCommunicationClient();
 
     ~GDBRemoteCommunicationClient() override;
 
     //------------------------------------------------------------------
     // After connecting, send the handshake to the server to make sure
     // we are communicating with it.
     //------------------------------------------------------------------
     bool
     HandshakeWithServer (Error *error_ptr);
 
-    PacketResult
-    SendPacketAndWaitForResponse (const char *send_payload,
-                                  StringExtractorGDBRemote &response,
-                                  bool send_async);
-
-    PacketResult
-    SendPacketAndWaitForResponse (const char *send_payload,
-                                  size_t send_length,
-                                  StringExtractorGDBRemote &response,
-                                  bool send_async);
-
     // For packets which specify a range of output to be returned,
     // return all of the output via a series of request packets of the form
     // <prefix>0,<size>
     // <prefix><size>,<size>
     // <prefix><size>*2,<size>
     // <prefix><size>*3,<size>
     // ...
     // until a "$l..." packet is received, indicating the end.
     // (size is in hex; this format is used by a standard gdbserver to
     // return the given portion of the output specified by <prefix>;
     // for example, "qXfer:libraries-svr4:read::fff,1000" means
     // "return a chunk of the xml description file for shared
     // library load addresses, where the chunk starts at offset 0xfff
     // and continues for 0x1000 bytes").
     // Concatenate the resulting server response packets together and
     // return in response_string.  If any packet fails, the return value
     // indicates that failure and the returned string value is undefined.
     PacketResult
     SendPacketsAndConcatenateResponses (const char *send_payload_prefix,
                                         std::string &response_string);
 
-    lldb::StateType
-    SendContinuePacketAndWaitForResponse (ProcessGDBRemote *process,
-                                          const char *packet_payload,
-                                          size_t packet_length,
-                                          StringExtractorGDBRemote &response);
-
-    bool
-    SendvContPacket (ProcessGDBRemote *process,
-                     const char *payload,
-                     size_t packet_length,
-                     StringExtractorGDBRemote &response);
-
     bool
     GetThreadSuffixSupported () override;
 
     // This packet is usually sent first and the boolean return value
     // indicates if the packet was send and any response was received
     // even in the response is UNIMPLEMENTED. If the packet failed to
     // get a response, then false is returned. This quickly tells us
     // if we were able to connect and communicate with the remote GDB
     // server
     bool
     QueryNoAckModeSupported ();
 
     void
     GetListThreadsInStopReplySupported ();
 
-    bool
-    SendAsyncSignal (int signo);
-
-    bool
-    SendInterrupt (Mutex::Locker &locker, 
-                   uint32_t seconds_to_wait_for_stop, 
-                   bool &timed_out);
-
     lldb::pid_t
     GetCurrentProcessID (bool allow_lazy = true);
 
     bool
     GetLaunchSuccess (std::string &error_str);
 
     bool
     LaunchGDBServer (const char *remote_accept_hostname,
SupportsAllocDeallocMemory () // const
         // m_supports_alloc_dealloc_memory = lldb_private::eLazyBoolNo;
         return m_supports_alloc_dealloc_memory;
     }
 
     size_t
     GetCurrentThreadIDs (std::vector<lldb::tid_t> &thread_ids,
                          bool &sequence_mutex_unavailable);
     
-    bool
-    GetInterruptWasSent () const
-    {
-        return m_interrupt_sent;
-    }
-    
     lldb::user_id_t
     OpenFile (const FileSpec& file_spec, uint32_t flags, mode_t mode, Error &error);
     
     bool
     CloseFile (lldb::user_id_t fd, Error &error);
     
     lldb::user_id_t
     GetFileSize (const FileSpec& file_spec);
RunShellCommand(const char *command,           // Shouldn't be nullptr
                     int *status_ptr,               // Pass nullptr if you don't want the process exit status
                     int *signo_ptr,                // Pass nullptr if you don't want the signal that caused the process to exit
                     std::string *command_output,   // Pass nullptr if you don't want the command output
                     uint32_t timeout_sec);         // Timeout in seconds to wait for shell program to finish
 
     bool
     CalculateMD5 (const FileSpec& file_spec, uint64_t &high, uint64_t &low);
     
-    std::string
-    HarmonizeThreadIdsForProfileData (ProcessGDBRemote *process,
-                                      StringExtractorGDBRemote &inputStringExtractor);
 
     bool
     ReadRegister(lldb::tid_t tid,
                  uint32_t reg_num,   // Must be the eRegisterKindProcessPlugin register number, to be sent to the remote
                  StringExtractorGDBRemote &response);
 
     bool
     ReadAllRegisters (lldb::tid_t tid,
bool
         m_supports_jThreadsInfo:1;
     
     lldb::pid_t m_curr_pid;
     lldb::tid_t m_curr_tid;         // Current gdb remote protocol thread index for all other operations
     lldb::tid_t m_curr_tid_run;     // Current gdb remote protocol thread index for continue, step, etc
 
     uint32_t m_num_supported_hardware_watchpoints;
 
-    // If we need to send a packet while the target is running, the m_async_XXX
-    // member variables take care of making this happen.
-    std::recursive_mutex m_async_mutex;
-    Predicate<bool> m_async_packet_predicate;
-    std::string m_async_packet;
-    PacketResult m_async_result;
-    StringExtractorGDBRemote m_async_response;
-    int m_async_signal; // We were asked to deliver a signal to the inferior process.
-    bool m_interrupt_sent;
-    std::string m_partial_profile_data;
-    std::map<uint64_t, uint32_t> m_thread_id_to_used_usec_map;
-    
     ArchSpec m_host_arch;
     ArchSpec m_process_arch;
     uint32_t m_os_version_major;
     uint32_t m_os_version_minor;
     uint32_t m_os_version_update;
     std::string m_os_build;
     std::string m_os_kernel;
     std::string m_hostname;
     std::string m_gdb_server_name; // from reply to qGDBServerVersion, empty if qGDBServerVersion is not supported
     uint32_t m_gdb_server_version; // from reply to qGDBServerVersion, zero if qGDBServerVersion is not supported
     uint32_t m_default_packet_timeout;
     uint64_t m_max_packet_size;  // as returned by qSupported
 
-    PacketResult
-    SendPacketAndWaitForResponseNoLock (const char *payload,
-                                        size_t payload_length,
-                                        StringExtractorGDBRemote &response);
-
     bool
     GetCurrentProcessInfo (bool allow_lazy_pid = true);
 
     bool
     GetGDBServerVersion();
 
     // Given the list of compression types that the remote debug stub can support,
     // possibly enable compression if we find an encoding we can handle.
     void
     MaybeEnableCompression (std::vector<std::string> supported_compressions);
 
     bool
     DecodeProcessInfoResponse (StringExtractorGDBRemote &response, 
                                ProcessInstanceInfo &process_info);
 
+    void
+    OnRunPacketSent(bool first) override;
+
 private:
     DISALLOW_COPY_AND_ASSIGN (GDBRemoteCommunicationClient);
 };
 
 } // namespace process_gdb_remote
 } // namespace lldb_private
 
 #endif // liblldb_GDBRemoteCommunicationClient_h_

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

 #include <math.h>
 #include <sys/stat.h>
 
 // C++ Includes
 #include <sstream>
 #include <numeric>
 
 // Other libraries and framework includes
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/Triple.h"
-#include "lldb/Interpreter/Args.h"
 #include "lldb/Core/Log.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/State.h"
 #include "lldb/Core/StreamGDBRemote.h"
 #include "lldb/Core/StreamString.h"
-#include "lldb/Host/ConnectionFileDescriptor.h"
-#include "lldb/Host/Endian.h"
-#include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/StringConvert.h"
 #include "lldb/Host/TimeValue.h"
+#include "lldb/Interpreter/Args.h"
 #include "lldb/Symbol/Symbol.h"
-#include "lldb/Target/Target.h"
 #include "lldb/Target/MemoryRegionInfo.h"
-#include "lldb/Target/UnixSignals.h"
+#include "lldb/Target/Target.h"
 
 // Project includes
 #include "Utility/StringExtractorGDBRemote.h"
 #include "ProcessGDBRemote.h"
 #include "ProcessGDBRemoteLog.h"
 #include "lldb/Host/Config.h"
 
 #if defined (HAVE_LIBCOMPRESSION)
 #include <compression.h>
 #endif
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
 
 //----------------------------------------------------------------------
 // GDBRemoteCommunicationClient constructor
 //----------------------------------------------------------------------
 GDBRemoteCommunicationClient::GDBRemoteCommunicationClient()
-    : GDBRemoteCommunication("gdb-remote.client", "gdb-remote.client.rx_packet"),
+    : GDBRemoteClientBase("gdb-remote.client", "gdb-remote.client.rx_packet"),
       m_supports_not_sending_acks(eLazyBoolCalculate),
       m_supports_thread_suffix(eLazyBoolCalculate),
       m_supports_threads_in_stop_reply(eLazyBoolCalculate),
       m_supports_vCont_all(eLazyBoolCalculate),
       m_supports_vCont_any(eLazyBoolCalculate),
       m_supports_vCont_c(eLazyBoolCalculate),
       m_supports_vCont_C(eLazyBoolCalculate),
       m_supports_vCont_s(eLazyBoolCalculate),
: GDBRemoteClientBase("gdb-remote.client", "gdb-remote.client.rx_packet"),
       m_supports_QSaveRegisterState(eLazyBoolCalculate),
       m_supports_qXfer_auxv_read(eLazyBoolCalculate),
       m_supports_qXfer_libraries_read(eLazyBoolCalculate),
       m_supports_qXfer_libraries_svr4_read(eLazyBoolCalculate),
       m_supports_qXfer_features_read(eLazyBoolCalculate),
       m_supports_augmented_libraries_svr4_read(eLazyBoolCalculate),
       m_supports_jThreadExtendedInfo(eLazyBoolCalculate),
       m_supports_jLoadedDynamicLibrariesInfos(eLazyBoolCalculate),
-      m_supports_jGetSharedCacheInfo (eLazyBoolCalculate),
+      m_supports_jGetSharedCacheInfo(eLazyBoolCalculate),
       m_supports_qProcessInfoPID(true),
       m_supports_qfProcessInfo(true),
       m_supports_qUserName(true),
       m_supports_qGroupName(true),
       m_supports_qThreadStopInfo(true),
       m_supports_z0(true),
       m_supports_z1(true),
       m_supports_z2(true),
       m_supports_z3(true),
       m_supports_z4(true),
       m_supports_QEnvironment(true),
       m_supports_QEnvironmentHexEncoded(true),
       m_supports_qSymbol(true),
       m_qSymbol_requests_done(false),
       m_supports_qModuleInfo(true),
       m_supports_jThreadsInfo(true),
       m_curr_pid(LLDB_INVALID_PROCESS_ID),
       m_curr_tid(LLDB_INVALID_THREAD_ID),
       m_curr_tid_run(LLDB_INVALID_THREAD_ID),
       m_num_supported_hardware_watchpoints(0),
-      m_async_mutex(),
-      m_async_packet_predicate(false),
-      m_async_packet(),
-      m_async_result(PacketResult::Success),
-      m_async_response(),
-      m_async_signal(-1),
-      m_interrupt_sent(false),
-      m_thread_id_to_used_usec_map(),
       m_host_arch(),
       m_process_arch(),
       m_os_version_major(UINT32_MAX),
       m_os_version_minor(UINT32_MAX),
       m_os_version_update(UINT32_MAX),
       m_os_build(),
       m_os_kernel(),
       m_hostname(),

 
 GDBRemoteCommunicationClient::PacketResult
 GDBRemoteCommunicationClient::SendPacketsAndConcatenateResponses
 (
     const char *payload_prefix,
     std::string &response_string
 )
 {
-    Mutex::Locker locker;
-    if (!GetSequenceMutex(locker,
-                          "ProcessGDBRemote::SendPacketsAndConcatenateResponses() failed due to not getting the sequence mutex"))
+    Lock lock(*this, false);
+    if (!lock)
     {
         Log *log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_PROCESS | GDBR_LOG_PACKETS));
         if (log)
             log->Printf("error: failed to get packet sequence mutex, not sending packets with prefix '%s'",
                         payload_prefix);
         return PacketResult::ErrorNoSequenceLock;
     }
 
     response_string = "";
     std::string payload_prefix_str(payload_prefix);
     unsigned int response_size = 0x1000;
     if (response_size > GetRemoteMaxPacketSize()) {  // May send qSupported packet
         response_size = GetRemoteMaxPacketSize();
     }
 
     for (unsigned int offset = 0; true; offset += response_size)
     {
         StringExtractorGDBRemote this_response;
         // Construct payload
         char sizeDescriptor[128];
         snprintf(sizeDescriptor, sizeof(sizeDescriptor), "%x,%x", offset, response_size);
-        PacketResult result = SendPacketAndWaitForResponse((payload_prefix_str + sizeDescriptor).c_str(),
-                                                           this_response,
-                                                           /*send_async=*/false);
+        PacketResult result = SendPacketAndWaitForResponseNoLock(payload_prefix_str + sizeDescriptor, this_response);
         if (result != PacketResult::Success)
             return result;
 
         const std::string &this_string = this_response.GetStringRef();
 
         // Check for m or l as first character; l seems to mean this is the last chunk
         char first_char = *this_string.c_str();
         if (first_char != 'm' && first_char != 'l')
         {
             return PacketResult::ErrorReplyInvalid;
         }
         // Concatenate the result so far (skipping 'm' or 'l')
         response_string.append(this_string, 1, std::string::npos);
         if (first_char == 'l')
             // We're done
             return PacketResult::Success;
     }
 }
 
-GDBRemoteCommunicationClient::PacketResult
-GDBRemoteCommunicationClient::SendPacketAndWaitForResponse
-(
-    const char *payload,
-    StringExtractorGDBRemote &response,
-    bool send_async
-)
-{
-    return SendPacketAndWaitForResponse (payload, 
-                                         ::strlen (payload),
-                                         response,
-                                         send_async);
-}
-
-GDBRemoteCommunicationClient::PacketResult
-GDBRemoteCommunicationClient::SendPacketAndWaitForResponseNoLock (const char *payload,
-                                                                  size_t payload_length,
-                                                                  StringExtractorGDBRemote &response)
-{
-    PacketResult packet_result = SendPacketNoLock(payload, payload_length);
-    if (packet_result == PacketResult::Success)
-    {
-        const size_t max_response_retries = 3;
-        for (size_t i=0; i<max_response_retries; ++i)
-        {
-            packet_result = ReadPacket(response, GetPacketTimeoutInMicroSeconds (), true);
-            // Make sure we received a response
-            if (packet_result != PacketResult::Success)
-                return packet_result;
-            // Make sure our response is valid for the payload that was sent
-            if (response.ValidateResponse())
-                return packet_result;
-            // Response says it wasn't valid
-            Log *log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS);
-            if (log)
-                log->Printf("error: packet with payload \"%*s\" got invalid response \"%s\": %s",
-                            (int)payload_length,
-                            payload,
-                            response.GetStringRef().c_str(),
-                            (i == (max_response_retries - 1)) ? "using invalid response and giving up" : "ignoring response and waiting for another");
-        }
-    }
-    return packet_result;
-}
-
-GDBRemoteCommunicationClient::PacketResult
-GDBRemoteCommunicationClient::SendPacketAndWaitForResponse
-(
-    const char *payload,
-    size_t payload_length,
-    StringExtractorGDBRemote &response,
-    bool send_async
-)
-{
-    PacketResult packet_result = PacketResult::ErrorSendFailed;
-    Mutex::Locker locker;
-    Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS));
-
-    // In order to stop async notifications from being processed in the middle of the
-    // send/receive sequence Hijack the broadcast. Then rebroadcast any events when we are done.
-    static ListenerSP hijack_listener_sp(Listener::MakeListener("lldb.NotifyHijacker"));
-    HijackBroadcaster(hijack_listener_sp, eBroadcastBitGdbReadThreadGotNotify);
-
-    if (GetSequenceMutex (locker))
-    {
-        packet_result = SendPacketAndWaitForResponseNoLock (payload, payload_length, response);
-    }
-    else
-    {
-        if (send_async)
-        {
-            if (IsRunning())
-            {
-                std::lock_guard<std::recursive_mutex> guard(m_async_mutex);
-                m_async_packet.assign(payload, payload_length);
-                m_async_response.CopyResponseValidator(response);
-                m_async_packet_predicate.SetValue (true, eBroadcastNever);
-                
-                if (log) 
-                    log->Printf ("async: async packet = %s", m_async_packet.c_str());
-
-                bool timed_out = false;
-                if (SendInterrupt(locker, 2, timed_out))
-                {
-                    if (m_interrupt_sent)
-                    {
-                        m_interrupt_sent = false;
-                        TimeValue timeout_time;
-                        timeout_time = TimeValue::Now();
-                        timeout_time.OffsetWithSeconds (m_packet_timeout);
-
-                        if (log) 
-                            log->Printf ("async: sent interrupt");
-
-                        if (m_async_packet_predicate.WaitForValueEqualTo (false, &timeout_time, &timed_out))
-                        {
-                            if (log) 
-                                log->Printf ("async: got response");
-
-                            // Swap the response buffer to avoid malloc and string copy
-                            response.GetStringRef().swap (m_async_response.GetStringRef());
-                            packet_result = m_async_result;
-                        }
-                        else
-                        {
-                            if (log) 
-                                log->Printf ("async: timed out waiting for response");
-                        }
-                        
-                        // Make sure we wait until the continue packet has been sent again...
-                        if (m_private_is_running.WaitForValueEqualTo (true, &timeout_time, &timed_out))
-                        {
-                            if (log)
-                            {
-                                if (timed_out) 
-                                    log->Printf ("async: timed out waiting for process to resume, but process was resumed");
-                                else
-                                    log->Printf ("async: async packet sent");
-                            }
-                        }
-                        else
-                        {
-                            if (log) 
-                                log->Printf ("async: timed out waiting for process to resume");
-                        }
-                    }
-                    else
-                    {
-                        // We had a racy condition where we went to send the interrupt
-                        // yet we were able to get the lock, so the process must have
-                        // just stopped?
-                        if (log) 
-                            log->Printf ("async: got lock without sending interrupt");
-                        // Send the packet normally since we got the lock
-                        packet_result = SendPacketAndWaitForResponseNoLock (payload, payload_length, response);
-                    }
-                }
-                else
-                {
-                    if (log) 
-                        log->Printf ("async: failed to interrupt");
-                }
-
-                m_async_response.SetResponseValidator(nullptr, nullptr);
-
-            }
-            else
-            {
-                if (log) 
-                    log->Printf ("async: not running, async is ignored");
-            }
-        }
-        else
-        {
-            if (log) 
-                log->Printf("error: failed to get packet sequence mutex, not sending packet '%*s'", (int) payload_length, payload);
-        }
-    }
-
-    // Remove our Hijacking listener from the broadcast.
-    RestoreBroadcaster();
-
-    // If a notification event occurred, rebroadcast since it can now be processed safely.
-    EventSP event_sp;
-    if (hijack_listener_sp->GetNextEvent(event_sp))
-        BroadcastEvent(event_sp);
-
-    return packet_result;
-}
-
-static const char *end_delimiter = "--end--;";
-static const int end_delimiter_len = 8;
-
-std::string
-GDBRemoteCommunicationClient::HarmonizeThreadIdsForProfileData
-(   ProcessGDBRemote *process,
-    StringExtractorGDBRemote& profileDataExtractor
-)
-{
-    std::map<uint64_t, uint32_t> new_thread_id_to_used_usec_map;
-    std::stringstream final_output;
-    std::string name, value;
-
-    // Going to assuming thread_used_usec comes first, else bail out.
-    while (profileDataExtractor.GetNameColonValue(name, value))
-    {
-        if (name.compare("thread_used_id") == 0)
-        {
-            StringExtractor threadIDHexExtractor(value.c_str());
-            uint64_t thread_id = threadIDHexExtractor.GetHexMaxU64(false, 0);
-            
-            bool has_used_usec = false;
-            uint32_t curr_used_usec = 0;
-            std::string usec_name, usec_value;
-            uint32_t input_file_pos = profileDataExtractor.GetFilePos();
-            if (profileDataExtractor.GetNameColonValue(usec_name, usec_value))
-            {
-                if (usec_name.compare("thread_used_usec") == 0)
-                {
-                    has_used_usec = true;
-                    curr_used_usec = strtoull(usec_value.c_str(), NULL, 0);
-                }
-                else
-                {
-                    // We didn't find what we want, it is probably
-                    // an older version. Bail out.
-                    profileDataExtractor.SetFilePos(input_file_pos);
-                }
-            }
-
-            if (has_used_usec)
-            {
-                uint32_t prev_used_usec = 0;
-                std::map<uint64_t, uint32_t>::iterator iterator = m_thread_id_to_used_usec_map.find(thread_id);
-                if (iterator != m_thread_id_to_used_usec_map.end())
-                {
-                    prev_used_usec = m_thread_id_to_used_usec_map[thread_id];
-                }
-                
-                uint32_t real_used_usec = curr_used_usec - prev_used_usec;
-                // A good first time record is one that runs for at least 0.25 sec
-                bool good_first_time = (prev_used_usec == 0) && (real_used_usec > 250000);
-                bool good_subsequent_time = (prev_used_usec > 0) &&
-                    ((real_used_usec > 0) || (process->HasAssignedIndexIDToThread(thread_id)));
-                
-                if (good_first_time || good_subsequent_time)
-                {
-                    // We try to avoid doing too many index id reservation,
-                    // resulting in fast increase of index ids.
-                    
-                    final_output << name << ":";
-                    int32_t index_id = process->AssignIndexIDToThread(thread_id);
-                    final_output << index_id << ";";
-                    
-                    final_output << usec_name << ":" << usec_value << ";";
-                }
-                else
-                {
-                    // Skip past 'thread_used_name'.
-                    std::string local_name, local_value;
-                    profileDataExtractor.GetNameColonValue(local_name, local_value);
-                }
-                
-                // Store current time as previous time so that they can be compared later.
-                new_thread_id_to_used_usec_map[thread_id] = curr_used_usec;
-            }
-            else
-            {
-                // Bail out and use old string.
-                final_output << name << ":" << value << ";";
-            }
-        }
-        else
-        {
-            final_output << name << ":" << value << ";";
-        }
-    }
-    final_output << end_delimiter;
-    m_thread_id_to_used_usec_map = new_thread_id_to_used_usec_map;
-    
-    return final_output.str();
-}
-
-bool
-GDBRemoteCommunicationClient::SendvContPacket
-(
-    ProcessGDBRemote *process,
-    const char *payload,
-    size_t packet_length,
-    StringExtractorGDBRemote &response
-)
-{
-
-    m_curr_tid = LLDB_INVALID_THREAD_ID;
-    Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
-    if (log)
-        log->Printf("GDBRemoteCommunicationClient::%s ()", __FUNCTION__);
-
-    // we want to lock down packet sending while we continue
-    Mutex::Locker locker(m_sequence_mutex);
-
-    // here we broadcast this before we even send the packet!!
-    // this signals doContinue() to exit
-    BroadcastEvent(eBroadcastBitRunPacketSent, NULL);
-
-    // set the public state to running
-    m_public_is_running.SetValue(true, eBroadcastNever);
-
-    // Set the starting continue packet into "continue_packet". This packet
-    // may change if we are interrupted and we continue after an async packet...
-    std::string continue_packet(payload, packet_length);
-
-    if (log)
-        log->Printf("GDBRemoteCommunicationClient::%s () sending vCont packet: %s", __FUNCTION__, continue_packet.c_str());
-
-    if (SendPacketNoLock(continue_packet.c_str(), continue_packet.size()) != PacketResult::Success)
-         return false;
-
-    // set the private state to running and broadcast this
-    m_private_is_running.SetValue(true, eBroadcastAlways);
-
-    if (log)
-        log->Printf("GDBRemoteCommunicationClient::%s () ReadPacket(%s)", __FUNCTION__, continue_packet.c_str());
-
-    // wait for the response to the vCont
-    if (ReadPacket(response, UINT32_MAX, false) == PacketResult::Success)
-    {
-        if (response.IsOKResponse())
-            return true;
-    }
-
-    return false;
-}
-
-StateType
-GDBRemoteCommunicationClient::SendContinuePacketAndWaitForResponse
-(
-    ProcessGDBRemote *process,
-    const char *payload,
-    size_t packet_length,
-    StringExtractorGDBRemote &response
-)
-{
-    m_curr_tid = LLDB_INVALID_THREAD_ID;
-    Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS));
-    if (log)
-        log->Printf ("GDBRemoteCommunicationClient::%s ()", __FUNCTION__);
-
-    Mutex::Locker locker(m_sequence_mutex);
-    StateType state = eStateRunning;
-
-    m_public_is_running.SetValue (true, eBroadcastNever);
-    // Set the starting continue packet into "continue_packet". This packet
-    // may change if we are interrupted and we continue after an async packet...
-    std::string continue_packet(payload, packet_length);
-
-    const auto sigstop_signo = process->GetUnixSignals()->GetSignalNumberFromName("SIGSTOP");
-    const auto sigint_signo = process->GetUnixSignals()->GetSignalNumberFromName("SIGINT");
-
-    bool got_async_packet = false;
-    bool broadcast_sent = false;
-    
-    while (state == eStateRunning)
-    {
-        if (!got_async_packet)
-        {
-            if (log)
-                log->Printf ("GDBRemoteCommunicationClient::%s () sending continue packet: %s", __FUNCTION__, continue_packet.c_str());
-            if (SendPacketNoLock(continue_packet.c_str(), continue_packet.size()) != PacketResult::Success)
-                state = eStateInvalid;
-            else
-                m_interrupt_sent = false;
-        
-            if (! broadcast_sent)
-            {
-                BroadcastEvent(eBroadcastBitRunPacketSent, NULL);
-                broadcast_sent = true;
-            }
-
-            m_private_is_running.SetValue (true, eBroadcastAlways);
-        }
-        
-        got_async_packet = false;
-
-        if (log)
-            log->Printf ("GDBRemoteCommunicationClient::%s () ReadPacket(%s)", __FUNCTION__, continue_packet.c_str());
-
-        if (ReadPacket(response, UINT32_MAX, false) == PacketResult::Success)
-        {
-            if (response.Empty())
-                state = eStateInvalid;
-            else
-            {
-                const char stop_type = response.GetChar();
-                if (log)
-                    log->Printf ("GDBRemoteCommunicationClient::%s () got packet: %s", __FUNCTION__, response.GetStringRef().c_str());
-                switch (stop_type)
-                {
-                case 'T':
-                case 'S':
-                    {
-                        if (process->GetStopID() == 0)
-                        {
-                            if (process->GetID() == LLDB_INVALID_PROCESS_ID)
-                            {
-                                lldb::pid_t pid = GetCurrentProcessID ();
-                                if (pid != LLDB_INVALID_PROCESS_ID)
-                                    process->SetID (pid);
-                            }
-                            process->BuildDynamicRegisterInfo (true);
-                        }
-
-                        // Privately notify any internal threads that we have stopped
-                        // in case we wanted to interrupt our process, yet we might
-                        // send a packet and continue without returning control to the
-                        // user.
-                        m_private_is_running.SetValue (false, eBroadcastAlways);
-
-                        const uint8_t signo = response.GetHexU8 (UINT8_MAX);
-
-                        bool continue_after_async = m_async_signal != -1 || m_async_packet_predicate.GetValue();
-                        if (continue_after_async || m_interrupt_sent)
-                        {
-                            // We sent an interrupt packet to stop the inferior process
-                            // for an async signal or to send an async packet while running
-                            // but we might have been single stepping and received the
-                            // stop packet for the step instead of for the interrupt packet.
-                            // Typically when an interrupt is sent a SIGINT or SIGSTOP
-                            // is used, so if we get anything else, we need to try and
-                            // get another stop reply packet that may have been sent
-                            // due to sending the interrupt when the target is stopped
-                            // which will just re-send a copy of the last stop reply
-                            // packet. If we don't do this, then the reply for our
-                            // async packet will be the repeat stop reply packet and cause
-                            // a lot of trouble for us! We also have some debugserver
-                            // binaries that would send two stop replies anytime the process
-                            // was interrupted, so we need to also check for an extra
-                            // stop reply packet if we interrupted the process
-                            const bool received_nonstop_signal = signo != sigint_signo && signo != sigstop_signo;
-                            if (m_interrupt_sent || received_nonstop_signal)
-                            {
-                                if (received_nonstop_signal)
-                                    continue_after_async = false;
-
-                                // Try for a very brief time (0.1s) to get another stop reply
-                                // packet to make sure it doesn't get in the way
-                                StringExtractorGDBRemote extra_stop_reply_packet;
-                                uint32_t timeout_usec = 100000;
-                                if (ReadPacket (extra_stop_reply_packet, timeout_usec, false) == PacketResult::Success)
-                                {
-                                    switch (extra_stop_reply_packet.GetChar())
-                                    {
-                                    case 'T':
-                                    case 'S':
-                                        // We did get an extra stop reply, which means
-                                        // our interrupt didn't stop the target so we
-                                        // shouldn't continue after the async signal
-                                        // or packet is sent...
-                                        continue_after_async = false;
-                                        break;
-                                    }
-                                }
-                            }
-                        }
-
-                        if (m_async_signal != -1)
-                        {
-                            if (log)
-                                log->Printf ("async: send signo = %s", Host::GetSignalAsCString (m_async_signal));
-
-                            // Save off the async signal we are supposed to send
-                            const int async_signal = m_async_signal;
-                            // Clear the async signal member so we don't end up
-                            // sending the signal multiple times...
-                            m_async_signal = -1;
-                            // Check which signal we stopped with
-                            if (signo == async_signal)
-                            {
-                                if (log) 
-                                    log->Printf ("async: stopped with signal %s, we are done running", Host::GetSignalAsCString (signo));
-
-                                // We already stopped with a signal that we wanted
-                                // to stop with, so we are done
-                            }
-                            else
-                            {
-                                // We stopped with a different signal that the one
-                                // we wanted to stop with, so now we must resume
-                                // with the signal we want
-                                char signal_packet[32];
-                                int signal_packet_len = 0;
-                                signal_packet_len = ::snprintf (signal_packet,
-                                                                sizeof (signal_packet),
-                                                                "C%2.2x",
-                                                                async_signal);
-
-                                if (log) 
-                                    log->Printf ("async: stopped with signal %s, resume with %s", 
-                                                       Host::GetSignalAsCString (signo),
-                                                       Host::GetSignalAsCString (async_signal));
-
-                                // Set the continue packet to resume even if the
-                                // interrupt didn't cause our stop (ignore continue_after_async)
-                                continue_packet.assign(signal_packet, signal_packet_len);
-                                continue;
-                            }
-                        }
-                        else if (m_async_packet_predicate.GetValue())
-                        {
-                            Log * packet_log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PACKETS));
-
-                            // We are supposed to send an asynchronous packet while
-                            // we are running.
-                            m_async_response.Clear();
-                            if (m_async_packet.empty())
-                            {
-                                m_async_result = PacketResult::ErrorSendFailed;
-                                if (packet_log)
-                                    packet_log->Printf ("async: error: empty async packet");                            
-
-                            }
-                            else
-                            {
-                                if (packet_log) 
-                                    packet_log->Printf ("async: sending packet");
-                                
-                                m_async_result = SendPacketAndWaitForResponse (&m_async_packet[0],
-                                                                               m_async_packet.size(),
-                                                                               m_async_response,
-                                                                               false);
-                            }
-                            // Let the other thread that was trying to send the async
-                            // packet know that the packet has been sent and response is
-                            // ready...
-                            m_async_packet_predicate.SetValue(false, eBroadcastAlways);
-
-                            if (packet_log) 
-                                packet_log->Printf ("async: sent packet, continue_after_async = %i", continue_after_async);
-
-                            // Set the continue packet to resume if our interrupt
-                            // for the async packet did cause the stop
-                            if (continue_after_async)
-                            {
-                                // Reverting this for now as it is causing deadlocks
-                                // in programs (<rdar://problem/11529853>). In the future
-                                // we should check our thread list and "do the right thing"
-                                // for new threads that show up while we stop and run async
-                                // packets. Setting the packet to 'c' to continue all threads
-                                // is the right thing to do 99.99% of the time because if a
-                                // thread was single stepping, and we sent an interrupt, we
-                                // will notice above that we didn't stop due to an interrupt
-                                // but stopped due to stepping and we would _not_ continue.
-                                continue_packet.assign (1, 'c');
-                                continue;
-                            }
-                        }
-                        // Stop with signal and thread info
-                        state = eStateStopped;
-                    }
-                    break;
-
-                case 'W':
-                case 'X':
-                    // process exited
-                    state = eStateExited;
-                    break;
-
-                case 'O':
-                    // STDOUT
-                    {
-                        got_async_packet = true;
-                        std::string inferior_stdout;
-                        inferior_stdout.reserve(response.GetBytesLeft () / 2);
-
-                        uint8_t ch;
-                        while (response.GetHexU8Ex(ch))
-                        {
-                            if (ch != 0)
-                                inferior_stdout.append(1, (char)ch);
-                        }
-                        process->AppendSTDOUT (inferior_stdout.c_str(), inferior_stdout.size());
-                    }
-                    break;
-
-                case 'A':
-                    // Async miscellaneous reply. Right now, only profile data is coming through this channel.
-                    {
-                        got_async_packet = true;
-                        std::string input = response.GetStringRef().substr(1); // '1' to move beyond 'A'
-                        if (m_partial_profile_data.length() > 0)
-                        {
-                            m_partial_profile_data.append(input);
-                            input = m_partial_profile_data;
-                            m_partial_profile_data.clear();
-                        }
-                        
-                        size_t found, pos = 0, len = input.length();
-                        while ((found = input.find(end_delimiter, pos)) != std::string::npos)
-                        {
-                            StringExtractorGDBRemote profileDataExtractor(input.substr(pos, found).c_str());
-                            std::string profile_data = HarmonizeThreadIdsForProfileData(process, profileDataExtractor);
-                            process->BroadcastAsyncProfileData (profile_data);
-                            
-                            pos = found + end_delimiter_len;
-                        }
-                        
-                        if (pos < len)
-                        {
-                            // Last incomplete chunk.
-                            m_partial_profile_data = input.substr(pos);
-                        }
-                    }
-                    break;
-
-                case 'E':
-                    // ERROR
-                    state = eStateInvalid;
-                    break;
-
-                default:
-                    if (log)
-                        log->Printf ("GDBRemoteCommunicationClient::%s () unrecognized async packet", __FUNCTION__);
-                    state = eStateInvalid;
-                    break;
-                }
-            }
-        }
-        else
-        {
-            if (log)
-                log->Printf ("GDBRemoteCommunicationClient::%s () ReadPacket(...) => false", __FUNCTION__);
-            state = eStateInvalid;
-        }
-    }
-    if (log)
-        log->Printf ("GDBRemoteCommunicationClient::%s () => %s", __FUNCTION__, StateAsCString(state));
-    response.SetFilePos(0);
-    m_private_is_running.SetValue (false, eBroadcastAlways);
-    m_public_is_running.SetValue (false, eBroadcastAlways);
-    return state;
-}
-
-bool
-GDBRemoteCommunicationClient::SendAsyncSignal (int signo)
-{
-    std::lock_guard<std::recursive_mutex> guard(m_async_mutex);
-    m_async_signal = signo;
-    bool timed_out = false;
-    Mutex::Locker locker;
-    if (SendInterrupt (locker, 1, timed_out))
-        return true;
-    m_async_signal = -1;
-    return false;
-}
-
-// This function takes a mutex locker as a parameter in case the GetSequenceMutex
-// actually succeeds. If it doesn't succeed in acquiring the sequence mutex 
-// (the expected result), then it will send the halt packet. If it does succeed
-// then the caller that requested the interrupt will want to keep the sequence
-// locked down so that no one else can send packets while the caller has control.
-// This function usually gets called when we are running and need to stop the 
-// target. It can also be used when we are running and we need to do something
-// else (like read/write memory), so we need to interrupt the running process
-// (gdb remote protocol requires this), and do what we need to do, then resume.
-
-bool
-GDBRemoteCommunicationClient::SendInterrupt
-(
-    Mutex::Locker& locker, 
-    uint32_t seconds_to_wait_for_stop,             
-    bool &timed_out
-)
-{
-    timed_out = false;
-    Log *log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_PROCESS | GDBR_LOG_PACKETS));
-
-    if (IsRunning())
-    {
-        // Only send an interrupt if our debugserver is running...
-        if (GetSequenceMutex (locker))
-        {
-            if (log)
-                log->Printf ("SendInterrupt () - got sequence mutex without having to interrupt");
-        }
-        else
-        {
-            // Someone has the mutex locked waiting for a response or for the
-            // inferior to stop, so send the interrupt on the down low...
-            char ctrl_c = '\x03';
-            ConnectionStatus status = eConnectionStatusSuccess;
-            size_t bytes_written = Write (&ctrl_c, 1, status, NULL);
-            if (log)
-                log->PutCString("send packet: \\x03");
-            if (bytes_written > 0)
-            {
-                m_interrupt_sent = true;
-                if (seconds_to_wait_for_stop)
-                {
-                    TimeValue timeout;
-                    if (seconds_to_wait_for_stop)
-                    {
-                        timeout = TimeValue::Now();
-                        timeout.OffsetWithSeconds (seconds_to_wait_for_stop);
-                    }
-                    if (m_private_is_running.WaitForValueEqualTo (false, &timeout, &timed_out))
-                    {
-                        if (log)
-                            log->PutCString ("SendInterrupt () - sent interrupt, private state stopped");
-                        return true;
-                    }
-                    else
-                    {
-                        if (log)
-                            log->Printf ("SendInterrupt () - sent interrupt, timed out wating for async thread resume");
-                    }
-                }
-                else
-                {
-                    if (log)
-                        log->Printf ("SendInterrupt () - sent interrupt, not waiting for stop...");
-                    return true;
-                }
-            }
-            else
-            {
-                if (log)
-                    log->Printf ("SendInterrupt () - failed to write interrupt");
-            }
-            return false;
-        }
-    }
-    else
-    {
-        if (log)
-            log->Printf ("SendInterrupt () - not running");
-    }
-    return true;
-}
 
 lldb::pid_t
 GDBRemoteCommunicationClient::GetCurrentProcessID (bool allow_lazy)
 {
     if (allow_lazy && m_curr_pid_is_valid == eLazyBoolYes)
         return m_curr_pid;
     
     // First try to retrieve the pid via the qProcessInfo request.
GDBRemoteCommunicationClient::SendGDBStoppointTypePacket (GDBStoppointType type, bool insert,  addr_t addr, uint32_t length)
     // Signal generic failure
     return UINT8_MAX;
 }
 
 size_t
 GDBRemoteCommunicationClient::GetCurrentThreadIDs (std::vector<lldb::tid_t> &thread_ids, 
                                                    bool &sequence_mutex_unavailable)
 {
-    Mutex::Locker locker;
     thread_ids.clear();
-    
-    if (GetSequenceMutex (locker, "ProcessGDBRemote::UpdateThreadList() failed due to not getting the sequence mutex"))
+    Lock lock(*this, false);
+    if (lock)
     {
         sequence_mutex_unavailable = false;
         StringExtractorGDBRemote response;
         
         PacketResult packet_result;
-        for (packet_result = SendPacketAndWaitForResponseNoLock ("qfThreadInfo", strlen("qfThreadInfo"), response);
+        for (packet_result = SendPacketAndWaitForResponseNoLock("qfThreadInfo", response);
              packet_result == PacketResult::Success && response.IsNormalResponse();
-             packet_result = SendPacketAndWaitForResponseNoLock ("qsThreadInfo", strlen("qsThreadInfo"), response))
+             packet_result = SendPacketAndWaitForResponseNoLock("qsThreadInfo", response))
         {
             char ch = response.GetChar();
             if (ch == 'l')
                 break;
             if (ch == 'm')
             {
                 do
                 {
#endif
         sequence_mutex_unavailable = true;
     }
     return thread_ids.size();
 }
 
 lldb::addr_t
 GDBRemoteCommunicationClient::GetShlibInfoAddr()
 {
-    if (!IsRunning())
+    Lock lock(*this, false);
+    if (lock)
     {
         StringExtractorGDBRemote response;
         if (SendPacketAndWaitForResponse("qShlibInfoAddr", ::strlen ("qShlibInfoAddr"), response, false) == PacketResult::Success)
         {
             if (response.IsNormalResponse())
                 return response.GetHexMaxU64(false, LLDB_INVALID_ADDRESS);
         }
     }
+    else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
+    {
+        log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for qShlibInfoAddr packet.",
+                    __FUNCTION__);
+    }
     return LLDB_INVALID_ADDRESS;
 }
 
 lldb_private::Error
 GDBRemoteCommunicationClient::RunShellCommand(const char *command,           // Shouldn't be NULL
                                               const FileSpec &working_dir,   // Pass empty FileSpec to use the current working directory
                                               int *status_ptr,               // Pass NULL if you don't want the process exit status
                                               int *signo_ptr,                // Pass NULL if you don't want the signal that caused the process to exit
if (m_avoid_g_packets == eLazyBoolCalculate)
         }
     }
     return m_avoid_g_packets == eLazyBoolYes;
 }
 
 bool
 GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg, StringExtractorGDBRemote &response)
 {
-    Mutex::Locker locker;
-    if (GetSequenceMutex (locker, "Didn't get sequence mutex for p packet."))
+    Lock lock(*this, false);
+    if (lock)
     {
         const bool thread_suffix_supported = GetThreadSuffixSupported();
         
         if (thread_suffix_supported || SetCurrentThread(tid))
         {
             char packet[64];
             int packet_len = 0;
             if (thread_suffix_supported)
                 packet_len = ::snprintf (packet, sizeof(packet), "p%x;thread:%4.4" PRIx64 ";", reg, tid);
             else
                 packet_len = ::snprintf (packet, sizeof(packet), "p%x", reg);
             assert (packet_len < ((int)sizeof(packet) - 1));
             return SendPacketAndWaitForResponse(packet, response, false) == PacketResult::Success;
         }
     }
+    else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
+    {
+        log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for p packet.", __FUNCTION__);
+    }
     return false;
 
 }
 
 
 bool
 GDBRemoteCommunicationClient::ReadAllRegisters (lldb::tid_t tid, StringExtractorGDBRemote &response)
 {
-    Mutex::Locker locker;
-    if (GetSequenceMutex (locker, "Didn't get sequence mutex for g packet."))
+    Lock lock(*this, false);
+    if (lock)
     {
         const bool thread_suffix_supported = GetThreadSuffixSupported();
 
         if (thread_suffix_supported || SetCurrentThread(tid))
         {
             char packet[64];
             int packet_len = 0;
             // Get all registers in one packet
             if (thread_suffix_supported)
                 packet_len = ::snprintf (packet, sizeof(packet), "g;thread:%4.4" PRIx64 ";", tid);
             else
                 packet_len = ::snprintf (packet, sizeof(packet), "g");
             assert (packet_len < ((int)sizeof(packet) - 1));
             return SendPacketAndWaitForResponse(packet, response, false) == PacketResult::Success;
         }
     }
+    else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
+    {
+        log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for g packet.", __FUNCTION__);
+    }
     return false;
 }
 bool
 GDBRemoteCommunicationClient::SaveRegisterState (lldb::tid_t tid, uint32_t &save_id)
 {
     save_id = 0; // Set to invalid save ID
     if (m_supports_QSaveRegisterState == eLazyBoolNo)
         return false;
     
     m_supports_QSaveRegisterState = eLazyBoolYes;
-    Mutex::Locker locker;
-    if (GetSequenceMutex (locker, "Didn't get sequence mutex for QSaveRegisterState."))
+    Lock lock(*this, false);
+    if (lock)
     {
         const bool thread_suffix_supported = GetThreadSuffixSupported();
         if (thread_suffix_supported || SetCurrentThread(tid))
         {
             char packet[256];
             if (thread_suffix_supported)
                 ::snprintf (packet, sizeof(packet), "QSaveRegisterState;thread:%4.4" PRIx64 ";", tid);
             else
if (lock)
                 if (response_save_id != 0)
                 {
                     save_id = response_save_id;
                     return true;
                 }
             }
         }
     }
+    else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
+    {
+        log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for QSaveRegisterState packet.",
+                    __FUNCTION__);
+    }
     return false;
 }
 
 bool
 GDBRemoteCommunicationClient::RestoreRegisterState (lldb::tid_t tid, uint32_t save_id)
 {
     // We use the "m_supports_QSaveRegisterState" variable here because the
     // QSaveRegisterState and QRestoreRegisterState packets must both be supported in
     // order to be useful
     if (m_supports_QSaveRegisterState == eLazyBoolNo)
         return false;
-    
-    Mutex::Locker locker;
-    if (GetSequenceMutex (locker, "Didn't get sequence mutex for QRestoreRegisterState."))
+    Lock lock(*this, false);
+    if (lock)
     {
         const bool thread_suffix_supported = GetThreadSuffixSupported();
         if (thread_suffix_supported || SetCurrentThread(tid))
         {
             char packet[256];
             if (thread_suffix_supported)
                 ::snprintf (packet, sizeof(packet), "QRestoreRegisterState:%u;thread:%4.4" PRIx64 ";", save_id, tid);
             else
if (lock)
                 {
                     // This packet isn't supported, don't try calling this packet or
                     // QSaveRegisterState again...
                     m_supports_QSaveRegisterState = eLazyBoolNo;
                 }
             }
         }
     }
+    else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
+    {
+        log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for QRestoreRegisterState packet.",
+                    __FUNCTION__);
+    }
     return false;
 }
 
 bool
 GDBRemoteCommunicationClient::GetModuleInfo (const FileSpec& module_file_spec,
                                              const lldb_private::ArchSpec& arch_spec,
                                              ModuleSpec &module_spec)
 {
GDBRemoteCommunicationClient::ServeSymbolLookups(lldb_private::Process *process)
     // symbols and we can stop asking.
     bool symbol_response_provided = false;
 
     // Is this the initial qSymbol:: packet?
     bool first_qsymbol_query = true;
 
     if (m_supports_qSymbol && m_qSymbol_requests_done == false)
     {
-        Mutex::Locker locker;
-        if (GetSequenceMutex(locker, "GDBRemoteCommunicationClient::ServeSymbolLookups() failed due to not getting the sequence mutex"))
+        Lock lock(*this, false);
+        if (lock)
         {
             StreamString packet;
             packet.PutCString ("qSymbol::");
             StringExtractorGDBRemote response;
-            while (SendPacketAndWaitForResponseNoLock(packet.GetData(), packet.GetSize(), response) == PacketResult::Success)
+            while (SendPacketAndWaitForResponseNoLock(packet.GetString(), response) == PacketResult::Success)
             {
                 if (response.IsOKResponse())
                 {
                     if (symbol_response_provided || first_qsymbol_query)
                     {
                         m_qSymbol_requests_done = true;
                     }
 
if (m_supports_qSymbol && m_qSymbol_requests_done == false)
                     }
                 }
             }
             // If we make it here, the symbol request packet response wasn't valid or
             // our symbol lookup failed so we must abort
             return;
 
         }
+        else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
+        {
+            log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex.", __FUNCTION__);
+        }
     }
 }
 
+void
+GDBRemoteCommunicationClient::OnRunPacketSent(bool first)
+{
+    GDBRemoteClientBase::OnRunPacketSent(first);
+    if (first)
+        BroadcastEvent(eBroadcastBitRunPacketSent, NULL);
+    m_curr_tid = LLDB_INVALID_THREAD_ID;
+}

source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

GDBRemoteRegisterContext::WriteRegisterBytes (const RegisterInfo *reg_info, DataExtractor &data, uint32_t data_offset)
     ExecutionContext exe_ctx (CalculateThread());
 
     Process *process = exe_ctx.GetProcessPtr();
     Thread *thread = exe_ctx.GetThreadPtr();
     if (process == NULL || thread == NULL)
         return false;
 
     GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote());
-// FIXME: This check isn't right because IsRunning checks the Public state, but this
-// is work you need to do - for instance in ShouldStop & friends - before the public
-// state has been changed.
-//    if (gdb_comm.IsRunning())
-//        return false;
-
 
 #if defined (LLDB_CONFIGURATION_DEBUG)
     assert (m_reg_data.GetByteSize() >= reg_info->byte_offset + reg_info->byte_size);
 #endif
 
     // If our register context and our register info disagree, which should never happen, don't
     // overwrite past the end of the buffer.
     if (m_reg_data.GetByteSize() < reg_info->byte_offset + reg_info->byte_size)
         return false;
 
     // Grab a pointer to where we are going to put this register
     uint8_t *dst = const_cast<uint8_t*>(m_reg_data.PeekData(reg_info->byte_offset, reg_info->byte_size));
 
     if (dst == NULL)
         return false;
 
 
     if (data.CopyByteOrderedData (data_offset,                  // src offset
                                   reg_info->byte_size,          // src length
                                   dst,                          // dst
                                   reg_info->byte_size,          // dst length
                                   m_reg_data.GetByteOrder()))   // dst byte order
     {
-        Mutex::Locker locker;
-        if (gdb_comm.GetSequenceMutex (locker, "Didn't get sequence mutex for write register."))
+        GDBRemoteClientBase::Lock lock(gdb_comm, false);
+        if (lock)
         {
             const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
             ProcessSP process_sp (m_thread.GetProcess());
             if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetProtocolID()))
             {
                 StreamString packet;
                 StringExtractorGDBRemote response;
                 
if (process == NULL || thread == NULL)
         return false;
 
     GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote());
 
     StringExtractorGDBRemote response;
 
     const bool use_g_packet = gdb_comm.AvoidGPackets ((ProcessGDBRemote *)process) == false;
 
-    Mutex::Locker locker;
-    if (gdb_comm.GetSequenceMutex (locker, "Didn't get sequence mutex for read all registers."))
+    GDBRemoteClientBase::Lock lock(gdb_comm, false);
+    if (lock)
     {
         SyncThreadState(process);
         
         char packet[32];
         const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
         ProcessSP process_sp (m_thread.GetProcess());
         if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetProtocolID()))
         {
GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data_sp)
     if (process == NULL || thread == NULL)
         return false;
 
     GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote());
 
     const bool use_g_packet = gdb_comm.AvoidGPackets ((ProcessGDBRemote *)process) == false;
 
     StringExtractorGDBRemote response;
-    Mutex::Locker locker;
-    if (gdb_comm.GetSequenceMutex (locker, "Didn't get sequence mutex for write all registers."))
+    GDBRemoteClientBase::Lock lock(gdb_comm, false);
+    if (lock)
     {
         const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
         ProcessSP process_sp (m_thread.GetProcess());
         if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetProtocolID()))
         {
             // The data_sp contains the entire G response packet including the
             // G, and if the thread suffix is supported, it has the thread suffix
             // as well.

source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

 #include "GDBRemoteCommunicationClient.h"
 #include "GDBRemoteRegisterContext.h"
 
 namespace lldb_private {
 namespace process_gdb_remote {
 
 class ThreadGDBRemote;
 
-class ProcessGDBRemote : public Process
+class ProcessGDBRemote : public Process, private GDBRemoteClientBase::ContinueDelegate
 {
 public:
     ProcessGDBRemote(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp);
 
     ~ProcessGDBRemote() override;
 
     static lldb::ProcessSP
     CreateInstance (lldb::TargetSP target_sp,
public:
     GetLoadedDynamicLibrariesInfos (const std::vector<lldb::addr_t> &load_addresses) override;
 
     StructuredData::ObjectSP
     GetLoadedDynamicLibrariesInfos_sender (StructuredData::ObjectSP args);
 
     StructuredData::ObjectSP
     GetSharedCacheInfo () override;
 
+    std::string
+    HarmonizeThreadIdsForProfileData(StringExtractorGDBRemote &inputStringExtractor);
+
 protected:
     friend class ThreadGDBRemote;
     friend class GDBRemoteCommunicationClient;
     friend class GDBRemoteRegisterContext;
 
     //------------------------------------------------------------------
     /// Broadcaster event bits definitions.
     //------------------------------------------------------------------
protected:
     lldb::ModuleSP
     LoadModuleAtAddress (const FileSpec &file, lldb::addr_t link_map, lldb::addr_t base_addr,
                          bool value_is_offset);
 
 private:
     //------------------------------------------------------------------
     // For ProcessGDBRemote only
     //------------------------------------------------------------------
+    std::string m_partial_profile_data;
+    std::map<uint64_t, uint32_t> m_thread_id_to_used_usec_map;
+
     static bool
     NewThreadNotifyBreakpointHit (void *baton,
                          StoppointCallbackContext *context,
                          lldb::user_id_t break_id,
                          lldb::user_id_t break_loc_id);
 
+    //------------------------------------------------------------------
+    // ContinueDelegate interface
+    //------------------------------------------------------------------
+    void
+    HandleAsyncStdout(llvm::StringRef out) override;
+    void
+    HandleAsyncMisc(llvm::StringRef data) override;
+    void
+    HandleStopReply() override;
+
     DISALLOW_COPY_AND_ASSIGN (ProcessGDBRemote);
 };
 
 } // namespace process_gdb_remote
 } // namespace lldb_private
 
 #endif // liblldb_ProcessGDBRemote_h_

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

 #include <sys/stat.h>
 #include <sys/types.h>
 #include <time.h>
 
 // C++ Includes
 #include <algorithm>
 #include <map>
 #include <mutex>
+#include <sstream>
 
 #include "lldb/Breakpoint/Watchpoint.h"
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileSpec.h"
 #include "lldb/Core/Module.h"

 
 }
 
 Error
 ProcessGDBRemote::DoHalt (bool &caused_stop)
 {
     Error error;
 
-    bool timed_out = false;
-    Mutex::Locker locker;
-
     if (m_public_state.GetValue() == eStateAttaching)
     {
         // We are being asked to halt during an attach. We need to just close
         // our file handle and debugserver will go away, and we can be done...
         m_gdb_comm.Disconnect();
     }
     else
     {
-        if (!m_gdb_comm.SendInterrupt (locker, 2, timed_out))
-        {
-            if (timed_out)
-                error.SetErrorString("timed out sending interrupt packet");
-            else
-                error.SetErrorString("unknown error sending interrupt packet");
-        }
-
-        caused_stop = m_gdb_comm.GetInterruptWasSent ();
+        caused_stop = m_gdb_comm.Interrupt();
     }
     return error;
 }
 
 Error
 ProcessGDBRemote::DoDetach(bool keep_stopped)
 {
     Error error;
while (!done)
                                 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))
+                                    if (!process->GetGDBRemote().SendvContPacket(
+                                            llvm::StringRef(continue_cstr, continue_cstr_len), response))
                                     {
                                         // 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);
+                                    StateType stop_state = process->GetGDBRemote().SendContinuePacketAndWaitForResponse(
+                                        *process, *process->GetUnixSignals(),
+                                        llvm::StringRef(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 ();
 
                                     switch (stop_state)
ProcessGDBRemote::ModulesDidLoad (ModuleList &module_list)
     // We must call the lldb_private::Process::ModulesDidLoad () first before we do anything
     Process::ModulesDidLoad (module_list);
 
     // After loading shared libraries, we can ask our remote GDB server if
     // it needs any symbols.
     m_gdb_comm.ServeSymbolLookups(this);
 }
 
+void
+ProcessGDBRemote::HandleAsyncStdout(llvm::StringRef out)
+{
+    AppendSTDOUT(out.data(), out.size());
+}
+
+static const char *end_delimiter = "--end--;";
+static const int end_delimiter_len = 8;
+
+void
+ProcessGDBRemote::HandleAsyncMisc(llvm::StringRef data)
+{
+    std::string input = data.str(); // '1' to move beyond 'A'
+    if (m_partial_profile_data.length() > 0)
+    {
+        m_partial_profile_data.append(input);
+        input = m_partial_profile_data;
+        m_partial_profile_data.clear();
+    }
+
+    size_t found, pos = 0, len = input.length();
+    while ((found = input.find(end_delimiter, pos)) != std::string::npos)
+    {
+        StringExtractorGDBRemote profileDataExtractor(input.substr(pos, found).c_str());
+        std::string profile_data = HarmonizeThreadIdsForProfileData(profileDataExtractor);
+        BroadcastAsyncProfileData(profile_data);
+
+        pos = found + end_delimiter_len;
+    }
+
+    if (pos < len)
+    {
+        // Last incomplete chunk.
+        m_partial_profile_data = input.substr(pos);
+    }
+}
+
+std::string
+ProcessGDBRemote::HarmonizeThreadIdsForProfileData(StringExtractorGDBRemote &profileDataExtractor)
+{
+    std::map<uint64_t, uint32_t> new_thread_id_to_used_usec_map;
+    std::stringstream final_output;
+    std::string name, value;
+
+    // Going to assuming thread_used_usec comes first, else bail out.
+    while (profileDataExtractor.GetNameColonValue(name, value))
+    {
+        if (name.compare("thread_used_id") == 0)
+        {
+            StringExtractor threadIDHexExtractor(value.c_str());
+            uint64_t thread_id = threadIDHexExtractor.GetHexMaxU64(false, 0);
+
+            bool has_used_usec = false;
+            uint32_t curr_used_usec = 0;
+            std::string usec_name, usec_value;
+            uint32_t input_file_pos = profileDataExtractor.GetFilePos();
+            if (profileDataExtractor.GetNameColonValue(usec_name, usec_value))
+            {
+                if (usec_name.compare("thread_used_usec") == 0)
+                {
+                    has_used_usec = true;
+                    curr_used_usec = strtoull(usec_value.c_str(), NULL, 0);
+                }
+                else
+                {
+                    // We didn't find what we want, it is probably
+                    // an older version. Bail out.
+                    profileDataExtractor.SetFilePos(input_file_pos);
+                }
+            }
+
+            if (has_used_usec)
+            {
+                uint32_t prev_used_usec = 0;
+                std::map<uint64_t, uint32_t>::iterator iterator = m_thread_id_to_used_usec_map.find(thread_id);
+                if (iterator != m_thread_id_to_used_usec_map.end())
+                {
+                    prev_used_usec = m_thread_id_to_used_usec_map[thread_id];
+                }
+
+                uint32_t real_used_usec = curr_used_usec - prev_used_usec;
+                // A good first time record is one that runs for at least 0.25 sec
+                bool good_first_time = (prev_used_usec == 0) && (real_used_usec > 250000);
+                bool good_subsequent_time =
+                    (prev_used_usec > 0) && ((real_used_usec > 0) || (HasAssignedIndexIDToThread(thread_id)));
+
+                if (good_first_time || good_subsequent_time)
+                {
+                    // We try to avoid doing too many index id reservation,
+                    // resulting in fast increase of index ids.
+
+                    final_output << name << ":";
+                    int32_t index_id = AssignIndexIDToThread(thread_id);
+                    final_output << index_id << ";";
+
+                    final_output << usec_name << ":" << usec_value << ";";
+                }
+                else
+                {
+                    // Skip past 'thread_used_name'.
+                    std::string local_name, local_value;
+                    profileDataExtractor.GetNameColonValue(local_name, local_value);
+                }
+
+                // Store current time as previous time so that they can be compared later.
+                new_thread_id_to_used_usec_map[thread_id] = curr_used_usec;
+            }
+            else
+            {
+                // Bail out and use old string.
+                final_output << name << ":" << value << ";";
+            }
+        }
+        else
+        {
+            final_output << name << ":" << value << ";";
+        }
+    }
+    final_output << end_delimiter;
+    m_thread_id_to_used_usec_map = new_thread_id_to_used_usec_map;
+
+    return final_output.str();
+}
+
+void
+ProcessGDBRemote::HandleStopReply()
+{
+    if (GetStopID() != 0)
+        return;
+
+    if (GetID() == LLDB_INVALID_PROCESS_ID)
+    {
+        lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
+        if (pid != LLDB_INVALID_PROCESS_ID)
+            SetID(pid);
+    }
+    BuildDynamicRegisterInfo(true);
+}
 
 class CommandObjectProcessGDBRemoteSpeedTest: public CommandObjectParsed
 {
 public:
     CommandObjectProcessGDBRemoteSpeedTest(CommandInterpreter &interpreter) :
         CommandObjectParsed (interpreter,
                              "process plugin packet speed-test",
                              "Tests packet speeds of various sizes to determine the performance characteristics of the GDB remote connection. ",
DoExecute (Args& command, CommandReturnObject &result) override
                 process->GetGDBRemote().SendPacketAndWaitForResponse(packet_cstr, response, send_async);
                 result.SetStatus (eReturnStatusSuccessFinishResult);
                 Stream &output_strm = result.GetOutputStream();
                 output_strm.Printf ("  packet: %s\n", packet_cstr);
                 std::string &response_str = response.GetStringRef();
 
                 if (strstr(packet_cstr, "qGetProfileData") != NULL)
                 {
-                    response_str = process->GetGDBRemote().HarmonizeThreadIdsForProfileData(process, response);
+                    response_str = process->HarmonizeThreadIdsForProfileData(response);
                 }
 
                 if (response_str.empty())
                     output_strm.PutCString ("response: \nerror: UNIMPLEMENTED\n");
                 else
                     output_strm.Printf ("response: %s\n", response.GetStringRef().c_str());
             }
         }

unittests/CMakeLists.txt

function(add_unittest_inputs test_name inputs)
   endforeach()
 endfunction()
 
 add_subdirectory(Core)
 add_subdirectory(Editline)
 add_subdirectory(Expression)
 add_subdirectory(Host)
 add_subdirectory(Interpreter)
+add_subdirectory(Process)
 add_subdirectory(ScriptInterpreter)
 add_subdirectory(Symbol)
 add_subdirectory(SymbolFile)
 add_subdirectory(Utility)

unittests/Process/CMakeLists.txt

+add_subdirectory(gdb-remote)

unittests/Process/gdb-remote/CMakeLists.txt

+add_lldb_unittest(ProcessGdbRemoteTests
+  GDBRemoteClientBaseTest.cpp
+  )

unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp

+//===-- GDBRemoteClientBaseTest.cpp -----------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#if defined(_MSC_VER) && (_HAS_EXCEPTIONS == 0)
+// Workaround for MSVC standard library bug, which fails to include <thread> when
+// exceptions are disabled.
+#include <eh.h>
+#endif
+#include <future>
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/LinuxSignals.h"
+#include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h"
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h"
+
+#include "lldb/Host/common/TCPSocket.h"
+#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
+
+#include "llvm/ADT/STLExtras.h"
+
+using namespace lldb_private::process_gdb_remote;
+using namespace lldb_private;
+using namespace lldb;
+typedef GDBRemoteCommunication::PacketResult PacketResult;
+
+namespace
+{
+
+struct MockDelegate : public GDBRemoteClientBase::ContinueDelegate
+{
+    std::string output;
+    std::string misc_data;
+    unsigned stop_reply_called = 0;
+
+    void
+    HandleAsyncStdout(llvm::StringRef out)
+    {
+        output += out;
+    }
+    void
+    HandleAsyncMisc(llvm::StringRef data)
+    {
+        misc_data += data;
+    }
+    void
+    HandleStopReply()
+    {
+        ++stop_reply_called;
+    }
+};
+
+struct MockServer : public GDBRemoteCommunicationServer
+{
+    MockServer() : GDBRemoteCommunicationServer("mock-server", "mock-server.listener") { m_send_acks = false; }
+
+    bool
+    GetThreadSuffixSupported() override
+    {
+        return false;
+    }
+
+    PacketResult
+    SendPacket(llvm::StringRef payload)
+    {
+        return GDBRemoteCommunicationServer::SendPacketNoLock(payload.data(), payload.size());
+    }
+
+    PacketResult
+    GetPacket(StringExtractorGDBRemote &response)
+    {
+        const unsigned timeout_usec = 1000000; // 1s
+        const bool sync_on_timeout = false;
+        return WaitForPacketWithTimeoutMicroSecondsNoLock(response, timeout_usec, sync_on_timeout);
+    }
+};
+
+struct TestClient : public GDBRemoteClientBase
+{
+    TestClient() : GDBRemoteClientBase("test.client", "test.client.listener") { m_send_acks = false; }
+
+    bool
+    GetThreadSuffixSupported() override
+    {
+        return false;
+    }
+};
+
+struct ContinueFixture
+{
+    MockDelegate delegate;
+    TestClient client;
+    MockServer server;
+    ListenerSP listener_sp;
+
+    ContinueFixture();
+
+    StateType
+    SendCPacket(StringExtractorGDBRemote &response)
+    {
+        return client.SendContinuePacketAndWaitForResponse(delegate, LinuxSignals(), "c", response);
+    }
+};
+
+ContinueFixture::ContinueFixture() : listener_sp(Listener::MakeListener("listener"))
+{
+    bool child_processes_inherit = false;
+    Error error;
+    TCPSocket listen_socket(child_processes_inherit, error);
+    EXPECT_FALSE(error.Fail());
+    error = listen_socket.Listen("127.0.0.1:0", 5);
+    EXPECT_FALSE(error.Fail());
+
+    Socket *accept_socket;
+    std::future<Error> accept_error = std::async(std::launch::async, [&] {
+        return listen_socket.Accept("127.0.0.1:0", child_processes_inherit, accept_socket);
+    });
+
+    char connect_remote_address[64];
+    snprintf(connect_remote_address, sizeof(connect_remote_address), "connect://localhost:%u",
+             listen_socket.GetLocalPortNumber());
+
+    std::unique_ptr<ConnectionFileDescriptor> conn_ap(new ConnectionFileDescriptor());
+    EXPECT_EQ(conn_ap->Connect(connect_remote_address, nullptr), lldb::eConnectionStatusSuccess);
+
+    client.SetConnection(conn_ap.release());
+    EXPECT_TRUE(accept_error.get().Success());
+    server.SetConnection(new ConnectionFileDescriptor(accept_socket));
+
+    listener_sp->StartListeningForEvents(&client, TestClient::eBroadcastBitRunPacketSent);
+}
+
+} // end anonymous namespace
+
+class GDBRemoteClientBaseTest : public testing::Test
+{
+public:
+    static void
+    SetUpTestCase()
+    {
+#if defined(_MSC_VER)
+        WSADATA data;
+        ::WSAStartup(MAKEWORD(2, 2), &data);
+#endif
+    }
+
+    static void
+    TearDownTestCase()
+    {
+#if defined(_MSC_VER)
+        ::WSACleanup();
+#endif
+    }
+};
+
+TEST(GDBRemoteClientBaseTest, SendContinueAndWait)
+{
+    StringExtractorGDBRemote response;
+    ContinueFixture fix;
+    if (HasFailure())
+        return;
+
+    // Continue. The inferior will stop with a signal.
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T01"));
+    ASSERT_EQ(eStateStopped, fix.SendCPacket(response));
+    ASSERT_EQ("T01", response.GetStringRef());
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("c", response.GetStringRef());
+
+    // Continue. The inferior will exit.
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("W01"));
+    ASSERT_EQ(eStateExited, fix.SendCPacket(response));
+    ASSERT_EQ("W01", response.GetStringRef());
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("c", response.GetStringRef());
+
+    // Continue. The inferior will get killed.
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("X01"));
+    ASSERT_EQ(eStateExited, fix.SendCPacket(response));
+    ASSERT_EQ("X01", response.GetStringRef());
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("c", response.GetStringRef());
+}
+
+TEST(GDBRemoteClientBaseTest, SendContinueAndAsyncSignal)
+{
+    StringExtractorGDBRemote continue_response, response;
+    ContinueFixture fix;
+    if (HasFailure())
+        return;
+
+    // SendAsyncSignal should do nothing when we are not running.
+    ASSERT_FALSE(fix.client.SendAsyncSignal(0x47));
+
+    // Continue. After the run packet is sent, send an async signal.
+    std::future<StateType> continue_state =
+        std::async(std::launch::async, [&] { return fix.SendCPacket(continue_response); });
+    EventSP event_sp;
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("c", response.GetStringRef());
+    fix.listener_sp->WaitForEventForBroadcasterWithType(nullptr, &fix.client, TestClient::eBroadcastBitRunPacketSent,
+                                                        event_sp);
+
+    std::future<bool> async_result = std::async(std::launch::async, [&] { return fix.client.SendAsyncSignal(0x47); });
+
+    // First we'll get interrupted.
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("\x03", response.GetStringRef());
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T13"));
+
+    // Then we get the signal packet.
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("C47", response.GetStringRef());
+    ASSERT_TRUE(async_result.get());
+
+    // And we report back a signal stop.
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T47"));
+    ASSERT_EQ(eStateStopped, continue_state.get());
+    ASSERT_EQ("T47", continue_response.GetStringRef());
+}
+
+TEST(GDBRemoteClientBaseTest, SendContinueAndAsyncPacket)
+{
+    StringExtractorGDBRemote continue_response, async_response, response;
+    const bool send_async = true;
+    ContinueFixture fix;
+    if (HasFailure())
+        return;
+
+    // Continue. After the run packet is sent, send an async packet.
+    std::future<StateType> continue_state =
+        std::async(std::launch::async, [&] { return fix.SendCPacket(continue_response); });
+    EventSP event_sp;
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("c", response.GetStringRef());
+    fix.listener_sp->WaitForEventForBroadcasterWithType(nullptr, &fix.client, TestClient::eBroadcastBitRunPacketSent,
+                                                        event_sp);
+
+    // Sending without async enabled should fail.
+    ASSERT_EQ(PacketResult::ErrorSendFailed, fix.client.SendPacketAndWaitForResponse("qTest1", response, !send_async));
+
+    std::future<PacketResult> async_result = std::async(std::launch::async, [&] {
+        return fix.client.SendPacketAndWaitForResponse("qTest2", async_response, send_async);
+    });
+
+    // First we'll get interrupted.
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("\x03", response.GetStringRef());
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T13"));
+
+    // Then we get the async packet.
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("qTest2", response.GetStringRef());
+
+    // Send the response and receive it.
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("QTest2"));
+    ASSERT_EQ(PacketResult::Success, async_result.get());
+    ASSERT_EQ("QTest2", async_response.GetStringRef());
+
+    // And we get resumed again.
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("c", response.GetStringRef());
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T01"));
+    ASSERT_EQ(eStateStopped, continue_state.get());
+    ASSERT_EQ("T01", continue_response.GetStringRef());
+}
+
+TEST(GDBRemoteClientBaseTest, SendContinueAndInterrupt)
+{
+    StringExtractorGDBRemote continue_response, response;
+    ContinueFixture fix;
+    if (HasFailure())
+        return;
+
+    // Interrupt should do nothing when we're not running.
+    ASSERT_FALSE(fix.client.Interrupt());
+
+    // Continue. After the run packet is sent, send an interrupt.
+    std::future<StateType> continue_state =
+        std::async(std::launch::async, [&] { return fix.SendCPacket(continue_response); });
+    EventSP event_sp;
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("c", response.GetStringRef());
+    fix.listener_sp->WaitForEventForBroadcasterWithType(nullptr, &fix.client, TestClient::eBroadcastBitRunPacketSent,
+                                                        event_sp);
+
+    std::future<bool> async_result = std::async(std::launch::async, [&] { return fix.client.Interrupt(); });
+
+    // We get interrupted.
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("\x03", response.GetStringRef());
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T13"));
+
+    // And that's it.
+    ASSERT_EQ(eStateStopped, continue_state.get());
+    ASSERT_EQ("T13", continue_response.GetStringRef());
+    ASSERT_TRUE(async_result.get());
+}
+
+TEST(GDBRemoteClientBaseTest, SendContinueAndInterrupt2PacketBug)
+{
+    StringExtractorGDBRemote continue_response, async_response, response;
+    const bool send_async = true;
+    ContinueFixture fix;
+    if (HasFailure())
+        return;
+
+    // Interrupt should do nothing when we're not running.
+    ASSERT_FALSE(fix.client.Interrupt());
+
+    // Continue. After the run packet is sent, send an async signal.
+    std::future<StateType> continue_state =
+        std::async(std::launch::async, [&] { return fix.SendCPacket(continue_response); });
+    EventSP event_sp;
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("c", response.GetStringRef());
+    fix.listener_sp->WaitForEventForBroadcasterWithType(nullptr, &fix.client, TestClient::eBroadcastBitRunPacketSent,
+                                                        event_sp);
+
+    std::future<bool> interrupt_result = std::async(std::launch::async, [&] { return fix.client.Interrupt(); });
+
+    // We get interrupted. We'll send two packets to simulate a buggy stub.
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("\x03", response.GetStringRef());
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T13"));
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T13"));
+
+    // We should stop.
+    ASSERT_EQ(eStateStopped, continue_state.get());
+    ASSERT_EQ("T13", continue_response.GetStringRef());
+    ASSERT_TRUE(interrupt_result.get());
+
+    // Packet stream should remain synchronized.
+    std::future<PacketResult> send_result = std::async(std::launch::async, [&] {
+        return fix.client.SendPacketAndWaitForResponse("qTest", async_response, !send_async);
+    });
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("qTest", response.GetStringRef());
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("QTest"));
+    ASSERT_EQ(PacketResult::Success, send_result.get());
+    ASSERT_EQ("QTest", async_response.GetStringRef());
+}
+
+TEST(GDBRemoteClientBaseTest, SendContinueDelegateInterface)
+{
+    StringExtractorGDBRemote response;
+    ContinueFixture fix;
+    if (HasFailure())
+        return;
+
+    // Continue. We'll have the server send a bunch of async packets before it stops.
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("O4142"));
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("Apro"));
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("O4344"));
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("Afile"));
+    ASSERT_EQ(PacketResult::Success, fix.server.SendPacket("T01"));
+    ASSERT_EQ(eStateStopped, fix.SendCPacket(response));
+    ASSERT_EQ("T01", response.GetStringRef());
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("c", response.GetStringRef());
+
+    EXPECT_EQ("ABCD", fix.delegate.output);
+    EXPECT_EQ("profile", fix.delegate.misc_data);
+    EXPECT_EQ(1u, fix.delegate.stop_reply_called);
+}
+
+TEST(GDBRemoteClientBaseTest, InterruptNoResponse)
+{
+    StringExtractorGDBRemote continue_response, response;
+    ContinueFixture fix;
+    if (HasFailure())
+        return;
+
+    // Continue. After the run packet is sent, send an interrupt.
+    std::future<StateType> continue_state =
+        std::async(std::launch::async, [&] { return fix.SendCPacket(continue_response); });
+    EventSP event_sp;
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("c", response.GetStringRef());
+    fix.listener_sp->WaitForEventForBroadcasterWithType(nullptr, &fix.client, TestClient::eBroadcastBitRunPacketSent,
+                                                        event_sp);
+
+    std::future<bool> async_result = std::async(std::launch::async, [&] { return fix.client.Interrupt(); });
+
+    // We get interrupted, but we don't send a stop packet.
+    ASSERT_EQ(PacketResult::Success, fix.server.GetPacket(response));
+    ASSERT_EQ("\x03", response.GetStringRef());
+
+    // The functions should still terminate (after a timeout).
+    ASSERT_TRUE(async_result.get());
+    ASSERT_EQ(eStateInvalid, continue_state.get());
+}