[Lldb-commits] [lldb] r242018 - Introduce a MainLoop class and switch llgs to use it

Pavel Labath labath at google.com
Mon Jul 13 03:44:56 PDT 2015


Author: labath
Date: Mon Jul 13 05:44:55 2015
New Revision: 242018

URL: http://llvm.org/viewvc/llvm-project?rev=242018&view=rev
Log:
Introduce a MainLoop class and switch llgs to use it

Summary:
This is the first part of our effort to make llgs single threaded. Currently, llgs consists of
about three threads and the synchronisation between them is a major source of latency when
debugging linux and android applications.

In order to be able to go single threaded, we must have the ability to listen for events from
multiple sources (primarily, client commands coming over the network and debug events from the
inferior) and perform necessary actions. For this reason I introduce the concept of a MainLoop.
A main loop has the ability to register callback's which will be invoked upon receipt of certain
events. MainLoopPosix has the ability to listen for file descriptors and signals.

For the moment, I have merely made the GDBRemoteCommunicationServerLLGS class use MainLoop
instead of waiting on the network socket directly, but the other threads still remain. In the
followup patches I indend to migrate NativeProcessLinux to this class and remove the remaining
threads.

Reviewers: ovyalov, clayborg, amccarth, zturner, emaste

Subscribers: tberghammer, lldb-commits

Differential Revision: http://reviews.llvm.org/D11066

Added:
    lldb/trunk/include/lldb/Host/MainLoop.h
    lldb/trunk/include/lldb/Host/MainLoopBase.h
    lldb/trunk/include/lldb/Host/posix/MainLoopPosix.h
    lldb/trunk/source/Host/posix/MainLoopPosix.cpp
Modified:
    lldb/trunk/include/lldb/Core/Connection.h
    lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
    lldb/trunk/lldb.xcodeproj/project.pbxproj
    lldb/trunk/source/Host/CMakeLists.txt
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
    lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
    lldb/trunk/tools/lldb-server/lldb-platform.cpp

Modified: lldb/trunk/include/lldb/Core/Connection.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Connection.h?rev=242018&r1=242017&r2=242018&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/Connection.h (original)
+++ lldb/trunk/include/lldb/Core/Connection.h Mon Jul 13 05:44:55 2015
@@ -187,6 +187,20 @@ public:
     virtual bool
     InterruptRead() = 0;
 
+    //------------------------------------------------------------------
+    /// Returns the underlying IOObject used by the Connection.
+    ///
+    /// The IOObject can be used to wait for data to become available
+    /// on the connection. If the Connection does not use IOObjects (and
+    /// hence does not support waiting) this function should return a
+    /// null pointer.
+    ///
+    /// @return
+    ///     The underlying IOObject used for reading.
+    //------------------------------------------------------------------
+    virtual lldb::IOObjectSP
+    GetReadObject() { return lldb::IOObjectSP(); }
+
 private:
     //------------------------------------------------------------------
     // For Connection only

Added: lldb/trunk/include/lldb/Host/MainLoop.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/MainLoop.h?rev=242018&view=auto
==============================================================================
--- lldb/trunk/include/lldb/Host/MainLoop.h (added)
+++ lldb/trunk/include/lldb/Host/MainLoop.h Mon Jul 13 05:44:55 2015
@@ -0,0 +1,26 @@
+//===-- MainLoop.h ----------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef lldb_Host_MainLoop_h_
+#define lldb_Host_MainLoop_h_
+
+#ifdef _WIN32
+namespace lldb_private
+{
+typedef MainLoopBase MainLoop;
+}
+#else
+#include "lldb/Host/posix/MainLoopPosix.h"
+namespace lldb_private
+{
+typedef MainLoopPosix MainLoop;
+}
+#endif
+
+#endif // lldb_Host_MainLoop_h_

Added: lldb/trunk/include/lldb/Host/MainLoopBase.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/MainLoopBase.h?rev=242018&view=auto
==============================================================================
--- lldb/trunk/include/lldb/Host/MainLoopBase.h (added)
+++ lldb/trunk/include/lldb/Host/MainLoopBase.h Mon Jul 13 05:44:55 2015
@@ -0,0 +1,94 @@
+//===-- MainLoopBase.h ------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef lldb_Host_posix_MainLoopBase_h_
+#define lldb_Host_posix_MainLoopBase_h_
+
+#include <functional>
+
+#include "llvm/Support/ErrorHandling.h"
+
+#include "lldb/Core/Error.h"
+#include "lldb/Host/IOObject.h"
+
+namespace lldb_private {
+
+// The purpose of this class is to enable multiplexed processing of data from different sources
+// without resorting to multi-threading. Clients can register IOObjects, which will be monitored
+// for readability, and when they become ready, the specified callback will be invoked.
+// Monitoring for writability is not supported, but can be easily added if needed.
+//
+// The RegisterReadObject function return a handle, which controls the duration of the monitoring. When
+// this handle is destroyed, the callback is deregistered.
+//
+// This class simply defines the interface common for all platforms, actual implementations are
+// platform-specific.
+class MainLoopBase
+{
+private:
+    class ReadHandle;
+
+public:
+    MainLoopBase() { }
+    virtual ~MainLoopBase() { }
+
+    typedef std::unique_ptr<ReadHandle> ReadHandleUP;
+
+    typedef std::function<void(MainLoopBase &)> Callback;
+
+    virtual ReadHandleUP
+    RegisterReadObject(const lldb::IOObjectSP &object_sp, const Callback &callback, Error &error)
+    { llvm_unreachable("Not implemented"); }
+
+    // Waits for registered events and invoke the proper callbacks. Returns when all callbacks
+    // deregister themselves or when someone requests termination.
+    virtual Error
+    Run()
+    { llvm_unreachable("Not implemented"); }
+
+    // Requests the exit of the Run() function.
+    virtual void
+    RequestTermination()
+    { llvm_unreachable("Not implemented"); }
+
+protected:
+    ReadHandleUP
+    CreateReadHandle(const lldb::IOObjectSP &object_sp)
+    { return ReadHandleUP(new ReadHandle(*this, object_sp)); }
+
+    virtual void
+    UnregisterReadObject(const lldb::IOObjectSP &object_sp)
+    { llvm_unreachable("Not implemented"); }
+
+private:
+    class ReadHandle
+    {
+    public:
+        ~ReadHandle() { m_mainloop.UnregisterReadObject(m_object_sp); }
+
+    private:
+        ReadHandle(MainLoopBase &mainloop, const lldb::IOObjectSP &object_sp)
+            : m_mainloop(mainloop), m_object_sp(object_sp)
+        { }
+
+        MainLoopBase &m_mainloop;
+        lldb::IOObjectSP m_object_sp;
+
+        friend class MainLoopBase;
+        DISALLOW_COPY_AND_ASSIGN(ReadHandle);
+    };
+
+private:
+    DISALLOW_COPY_AND_ASSIGN(MainLoopBase);
+};
+
+} // namespace lldb_private
+
+
+#endif // lldb_Host_posix_MainLoopBase_h_

Modified: lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h?rev=242018&r1=242017&r2=242018&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h (original)
+++ lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h Mon Jul 13 05:44:55 2015
@@ -59,12 +59,7 @@ class ConnectionFileDescriptor : public
     bool InterruptRead() override;
 
     lldb::IOObjectSP
-    GetReadObject()
-    {
-        return m_read_sp;
-    }
-    const lldb::IOObjectSP
-    GetReadObject() const
+    GetReadObject() override
     {
         return m_read_sp;
     }

Added: lldb/trunk/include/lldb/Host/posix/MainLoopPosix.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/posix/MainLoopPosix.h?rev=242018&view=auto
==============================================================================
--- lldb/trunk/include/lldb/Host/posix/MainLoopPosix.h (added)
+++ lldb/trunk/include/lldb/Host/posix/MainLoopPosix.h Mon Jul 13 05:44:55 2015
@@ -0,0 +1,100 @@
+//===-- MainLoopPosix.h -----------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef lldb_Host_posix_MainLoopPosix_h_
+#define lldb_Host_posix_MainLoopPosix_h_
+
+#include "lldb/Host/MainLoopBase.h"
+
+#include "llvm/ADT/DenseMap.h"
+
+namespace lldb_private {
+
+// Posix implementation of the MainLoopBase class. It can monitor file descriptors for
+// readability using pselect. In addition to the common base, this class provides the ability to
+// invoke a given handler when a signal is received.
+//
+// Since this class is primarily intended to be used for single-threaded processing, it does not
+// attempt to perform any internal synchronisation and any concurrent accesses must be protected
+// externally. However, it is perfectly legitimate to have more than one instance of this class
+// running on separate threads, or even a single thread (with some limitations on signal
+// monitoring).
+// TODO: Add locking if this class is to be used in a multi-threaded context.
+class MainLoopPosix: public MainLoopBase
+{
+private:
+    class SignalHandle;
+
+public:
+    typedef std::unique_ptr<SignalHandle> SignalHandleUP;
+
+    ~MainLoopPosix() override;
+
+    ReadHandleUP
+    RegisterReadObject(const lldb::IOObjectSP &object_sp, const Callback &callback, Error &error) override;
+
+    // Listening for signals from multiple MainLoopPosix instances is perfectly safe as long as they
+    // don't try to listen for the same signal. The callback function is invoked when the control
+    // returns to the Run() function, not when the hander is executed. This means that you can
+    // treat the callback as a normal function and perform things which would not be safe in a
+    // signal handler. However, since the callback is not invoked synchronously, you cannot use
+    // this mechanism to handle SIGSEGV and the like.
+    SignalHandleUP
+    RegisterSignal(int signo, const Callback &callback, Error &error);
+
+    Error
+    Run() override;
+
+    // This should only be performed from a callback. Do not attempt to terminate the processing
+    // from another thread.
+    // TODO: Add synchronization if we want to be terminated from another thread.
+    void
+    RequestTermination() override
+    { m_terminate_request = true; }
+
+protected:
+    void
+    UnregisterReadObject(const lldb::IOObjectSP &object_sp) override;
+
+    void
+    UnregisterSignal(int signo);
+
+private:
+    class SignalHandle
+    {
+    public:
+        ~SignalHandle() { m_mainloop.UnregisterSignal(m_signo); }
+
+    private:
+        SignalHandle(MainLoopPosix &mainloop, int signo) : m_mainloop(mainloop), m_signo(signo) { }
+
+        MainLoopPosix &m_mainloop;
+        int m_signo;
+
+        friend class MainLoopPosix;
+        DISALLOW_COPY_AND_ASSIGN(SignalHandle);
+    };
+
+    struct SignalInfo
+    {
+        Callback callback;
+        struct sigaction old_action;
+        bool was_blocked : 1;
+    };
+
+    llvm::DenseMap<IOObject::WaitableHandle, Callback> m_read_fds;
+    llvm::DenseMap<int, SignalInfo> m_signals;
+    bool m_terminate_request : 1;
+};
+
+} // namespace lldb_private
+
+
+#endif // lldb_Host_posix_MainLoopPosix_h_
+

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=242018&r1=242017&r2=242018&view=diff
==============================================================================
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Mon Jul 13 05:44:55 2015
@@ -632,6 +632,7 @@
 		26FFC19914FC072100087D58 /* AuxVector.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26FFC19314FC072100087D58 /* AuxVector.cpp */; };
 		26FFC19B14FC072100087D58 /* DYLDRendezvous.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26FFC19514FC072100087D58 /* DYLDRendezvous.cpp */; };
 		26FFC19D14FC072100087D58 /* DynamicLoaderPOSIXDYLD.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26FFC19714FC072100087D58 /* DynamicLoaderPOSIXDYLD.cpp */; };
+		30DED5DE1B4ECB49004CC508 /* MainLoopPosix.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 30DED5DC1B4ECB17004CC508 /* MainLoopPosix.cpp */; };
 		332CCB181AFF41620034D4C4 /* SBLanguageRuntime.h in Headers */ = {isa = PBXBuildFile; fileRef = 3392EBB71AFF402200858B9F /* SBLanguageRuntime.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		33E5E8421A672A240024ED68 /* StringConvert.cpp in CopyFiles */ = {isa = PBXBuildFile; fileRef = 33E5E8411A672A240024ED68 /* StringConvert.cpp */; };
 		33E5E8461A6736D30024ED68 /* StringConvert.h in CopyFiles */ = {isa = PBXBuildFile; fileRef = 33E5E8451A6736D30024ED68 /* StringConvert.h */; };
@@ -2116,6 +2117,7 @@
 		26FFC19614FC072100087D58 /* DYLDRendezvous.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DYLDRendezvous.h; sourceTree = "<group>"; };
 		26FFC19714FC072100087D58 /* DynamicLoaderPOSIXDYLD.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DynamicLoaderPOSIXDYLD.cpp; sourceTree = "<group>"; };
 		26FFC19814FC072100087D58 /* DynamicLoaderPOSIXDYLD.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DynamicLoaderPOSIXDYLD.h; sourceTree = "<group>"; };
+		30DED5DC1B4ECB17004CC508 /* MainLoopPosix.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MainLoopPosix.cpp; sourceTree = "<group>"; };
 		33064C991A5C7A330033D415 /* UriParser.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = UriParser.cpp; path = source/Utility/UriParser.cpp; sourceTree = "<group>"; };
 		33064C9B1A5C7A490033D415 /* UriParser.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = UriParser.h; path = source/Utility/UriParser.h; sourceTree = "<group>"; };
 		3392EBB71AFF402200858B9F /* SBLanguageRuntime.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SBLanguageRuntime.h; path = include/lldb/API/SBLanguageRuntime.h; sourceTree = "<group>"; };
@@ -4891,6 +4893,7 @@
 			isa = PBXGroup;
 			children = (
 				255EFF751AFABA950069F277 /* LockFilePosix.cpp */,
+				30DED5DC1B4ECB17004CC508 /* MainLoopPosix.cpp */,
 				AFDFDFD019E34D3400EAE509 /* ConnectionFileDescriptorPosix.cpp */,
 				3FDFDDC5199D37ED009756A7 /* FileSystem.cpp */,
 				3FDFE53019A292F0009756A7 /* HostInfoPosix.cpp */,
@@ -6565,6 +6568,7 @@
 			isa = PBXSourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				30DED5DE1B4ECB49004CC508 /* MainLoopPosix.cpp in Sources */,
 				E769331C1A94D15400C73337 /* lldb-gdbserver.cpp in Sources */,
 				26DC6A1D1337FECA00FF7998 /* lldb-platform.cpp in Sources */,
 				E769331E1A94D18100C73337 /* lldb-server.cpp in Sources */,

Modified: lldb/trunk/source/Host/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/CMakeLists.txt?rev=242018&r1=242017&r2=242018&view=diff
==============================================================================
--- lldb/trunk/source/Host/CMakeLists.txt (original)
+++ lldb/trunk/source/Host/CMakeLists.txt Mon Jul 13 05:44:55 2015
@@ -76,6 +76,7 @@ else()
     posix/HostProcessPosix.cpp
     posix/HostThreadPosix.cpp
     posix/LockFilePosix.cpp
+    posix/MainLoopPosix.cpp
     posix/PipePosix.cpp
     )
 

Added: lldb/trunk/source/Host/posix/MainLoopPosix.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/MainLoopPosix.cpp?rev=242018&view=auto
==============================================================================
--- lldb/trunk/source/Host/posix/MainLoopPosix.cpp (added)
+++ lldb/trunk/source/Host/posix/MainLoopPosix.cpp Mon Jul 13 05:44:55 2015
@@ -0,0 +1,193 @@
+//===-- MainLoopPosix.cpp ---------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/posix/MainLoopPosix.h"
+
+#include <vector>
+
+#include "lldb/Core/Error.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static sig_atomic_t g_signal_flags[NSIG];
+
+static void
+SignalHandler(int signo, siginfo_t *info, void *)
+{
+    assert(signo < NSIG);
+    g_signal_flags[signo] = 1;
+}
+
+
+MainLoopPosix::~MainLoopPosix()
+{
+    assert(m_read_fds.size() == 0);
+    assert(m_signals.size() == 0);
+}
+
+MainLoopPosix::ReadHandleUP
+MainLoopPosix::RegisterReadObject(const IOObjectSP &object_sp, const Callback &callback, Error &error)
+{
+    if (!object_sp || !object_sp->IsValid())
+    {
+        error.SetErrorString("IO object is not valid.");
+        return nullptr;
+    }
+
+    const bool inserted = m_read_fds.insert({object_sp->GetWaitableHandle(), callback}).second;
+    if (! inserted)
+    {
+        error.SetErrorStringWithFormat("File descriptor %d already monitored.",
+                object_sp->GetWaitableHandle());
+        return nullptr;
+    }
+
+    return CreateReadHandle(object_sp);
+}
+
+// We shall block the signal, then install the signal handler. The signal will be unblocked in
+// the Run() function to check for signal delivery.
+MainLoopPosix::SignalHandleUP
+MainLoopPosix::RegisterSignal(int signo, const Callback &callback, Error &error)
+{
+    if (m_signals.find(signo) != m_signals.end())
+    {
+        error.SetErrorStringWithFormat("Signal %d already monitored.", signo);
+        return nullptr;
+    }
+
+    SignalInfo info;
+    info.callback = callback;
+    struct sigaction new_action;
+    new_action.sa_sigaction = &SignalHandler;
+    new_action.sa_flags = SA_SIGINFO;
+    sigemptyset(&new_action.sa_mask);
+    sigaddset(&new_action.sa_mask, signo);
+
+    sigset_t old_set;
+    if (int ret = pthread_sigmask(SIG_BLOCK, &new_action.sa_mask, &old_set))
+    {
+        error.SetErrorStringWithFormat("pthread_sigmask failed with error %d\n", ret);
+        return nullptr;
+    }
+
+    info.was_blocked = sigismember(&old_set, signo);
+    if (sigaction(signo, &new_action, &info.old_action) == -1)
+    {
+        error.SetErrorToErrno();
+        if (!info.was_blocked)
+            pthread_sigmask(SIG_UNBLOCK, &new_action.sa_mask, nullptr);
+        return nullptr;
+    }
+
+    m_signals.insert({signo, info});
+    g_signal_flags[signo] = 0;
+
+    return SignalHandleUP(new SignalHandle(*this, signo));
+}
+
+void
+MainLoopPosix::UnregisterReadObject(const lldb::IOObjectSP &object_sp)
+{
+    bool erased = m_read_fds.erase(object_sp->GetWaitableHandle());
+    (void) erased;
+    assert(erased);
+}
+
+void
+MainLoopPosix::UnregisterSignal(int signo)
+{
+    // We undo the actions of RegisterSignal on a best-effort basis.
+    auto it = m_signals.find(signo);
+    assert(it != m_signals.end());
+
+    sigaction(signo, &it->second.old_action, nullptr);
+
+    sigset_t set;
+    sigemptyset(&set);
+    sigaddset(&set, signo);
+    pthread_sigmask(it->second.was_blocked ? SIG_BLOCK : SIG_UNBLOCK, &set, nullptr);
+
+    m_signals.erase(it);
+}
+
+Error
+MainLoopPosix::Run()
+{
+    std::vector<int> signals;
+    sigset_t sigmask;
+    std::vector<int> read_fds;
+    fd_set read_fd_set;
+    m_terminate_request = false;
+
+    // run until termination or until we run out of things to listen to
+    while (! m_terminate_request && (!m_read_fds.empty() || !m_signals.empty()))
+    {
+        // To avoid problems with callbacks changing the things we're supposed to listen to, we
+        // will store the *real* list of events separately.
+        signals.clear();
+        read_fds.clear();
+        FD_ZERO(&read_fd_set);
+        int nfds = 0;
+
+        if (int ret = pthread_sigmask(SIG_SETMASK, nullptr, &sigmask))
+            return Error("pthread_sigmask failed with error %d\n", ret);
+
+        for (const auto &fd: m_read_fds)
+        {
+            read_fds.push_back(fd.first);
+            FD_SET(fd.first, &read_fd_set);
+            nfds = std::max(nfds, fd.first+1);
+        }
+
+        for (const auto &sig: m_signals)
+        {
+            signals.push_back(sig.first);
+            sigdelset(&sigmask, sig.first);
+        }
+
+        if (pselect(nfds, &read_fd_set, nullptr, nullptr, nullptr, &sigmask) == -1 && errno != EINTR)
+            return Error(errno, eErrorTypePOSIX);
+
+        for (int sig: signals)
+        {
+            if (g_signal_flags[sig] == 0)
+                continue; // No signal
+            g_signal_flags[sig] = 0;
+
+            auto it = m_signals.find(sig);
+            if (it == m_signals.end())
+                continue; // Signal must have gotten unregistered in the meantime
+
+            it->second.callback(*this); // Do the work
+
+            if (m_terminate_request)
+                return Error();
+        }
+
+        for (int fd: read_fds)
+        {
+            if (!FD_ISSET(fd, &read_fd_set))
+                continue; // Not ready
+
+            auto it = m_read_fds.find(fd);
+            if (it == m_read_fds.end())
+                continue; // File descriptor must have gotten unregistered in the meantime
+
+            it->second(*this); // Do the work
+
+            if (m_terminate_request)
+                return Error();
+        }
+    }
+    return Error();
+}
+
+

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp?rev=242018&r1=242017&r2=242018&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp Mon Jul 13 05:44:55 2015
@@ -132,7 +132,7 @@ GDBRemoteCommunicationServer::SendOKResp
 }
 
 bool
-GDBRemoteCommunicationServer::HandshakeWithClient(Error *error_ptr)
+GDBRemoteCommunicationServer::HandshakeWithClient()
 {
     return GetAck() == PacketResult::Success;
 }

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h?rev=242018&r1=242017&r2=242018&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h Mon Jul 13 05:44:55 2015
@@ -54,7 +54,7 @@ public:
     // After connecting, do a little handshake with the client to make sure
     // we are at least communicating
     bool
-    HandshakeWithClient (Error *error_ptr);
+    HandshakeWithClient ();
 
 protected:
     std::map<StringExtractorGDBRemote::ServerPacketType, PacketHandler> m_packet_handlers;

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=242018&r1=242017&r2=242018&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp Mon Jul 13 05:44:55 2015
@@ -75,10 +75,11 @@ namespace
 // GDBRemoteCommunicationServerLLGS constructor
 //----------------------------------------------------------------------
 GDBRemoteCommunicationServerLLGS::GDBRemoteCommunicationServerLLGS(
-        const lldb::PlatformSP& platform_sp) :
+        const lldb::PlatformSP& platform_sp,
+        MainLoop &mainloop) :
     GDBRemoteCommunicationServerCommon ("gdb-remote.server", "gdb-remote.server.rx_packet"),
     m_platform_sp (platform_sp),
-    m_async_thread (LLDB_INVALID_HOST_THREAD),
+    m_mainloop (mainloop),
     m_current_tid (LLDB_INVALID_THREAD_ID),
     m_continue_tid (LLDB_INVALID_THREAD_ID),
     m_debugged_process_mutex (Mutex::eMutexTypeRecursive),
@@ -88,7 +89,8 @@ GDBRemoteCommunicationServerLLGS::GDBRem
     m_active_auxv_buffer_sp (),
     m_saved_registers_mutex (),
     m_saved_registers_map (),
-    m_next_saved_registers_id (1)
+    m_next_saved_registers_id (1),
+    m_handshake_completed (false)
 {
     assert(platform_sp);
     RegisterPacketHandlers();
@@ -782,6 +784,58 @@ GDBRemoteCommunicationServerLLGS::DidExe
     ClearProcessSpecificData ();
 }
 
+void
+GDBRemoteCommunicationServerLLGS::DataAvailableCallback ()
+{
+    Log *log (GetLogIfAnyCategoriesSet(GDBR_LOG_COMM));
+
+    if (! m_handshake_completed)
+    {
+        if (! HandshakeWithClient())
+        {
+            if(log)
+                log->Printf("GDBRemoteCommunicationServerLLGS::%s handshake with client failed, exiting",
+                        __FUNCTION__);
+            m_read_handle_up.reset();
+            m_mainloop.RequestTermination();
+            return;
+        }
+        m_handshake_completed = true;
+    }
+
+    bool interrupt = false;
+    bool done = false;
+    Error error;
+    while (true)
+    {
+        const PacketResult result = GetPacketAndSendResponse (0, error, interrupt, done);
+        if (result == PacketResult::ErrorReplyTimeout)
+            break; // No more packets in the queue
+
+        if ((result != PacketResult::Success))
+        {
+            if(log)
+                log->Printf("GDBRemoteCommunicationServerLLGS::%s processing a packet failed: %s",
+                        __FUNCTION__, error.AsCString());
+            m_read_handle_up.reset();
+            m_mainloop.RequestTermination();
+            break;
+        }
+    }
+}
+
+Error
+GDBRemoteCommunicationServerLLGS::InitializeConnection (std::unique_ptr<Connection> &&connection)
+{
+    IOObjectSP read_object_sp = connection->GetReadObject();
+    GDBRemoteCommunicationServer::SetConnection(connection.release());
+
+    Error error;
+    m_read_handle_up = m_mainloop.RegisterReadObject(read_object_sp,
+            [this] (MainLoopBase &) { DataAvailableCallback(); }, error);
+    return error;
+}
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::SendONotification (const char *buffer, uint32_t len)
 {

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h?rev=242018&r1=242017&r2=242018&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h Mon Jul 13 05:44:55 2015
@@ -19,6 +19,7 @@
 #include "lldb/Core/Communication.h"
 #include "lldb/Host/Mutex.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
+#include "lldb/Host/MainLoop.h"
 
 // Project includes
 #include "GDBRemoteCommunicationServerCommon.h"
@@ -26,6 +27,7 @@
 class StringExtractorGDBRemote;
 
 namespace lldb_private {
+
 namespace process_gdb_remote {
 
 class ProcessGDBRemote;
@@ -38,7 +40,7 @@ public:
     //------------------------------------------------------------------
     // Constructors and Destructors
     //------------------------------------------------------------------
-    GDBRemoteCommunicationServerLLGS(const lldb::PlatformSP& platform_sp);
+    GDBRemoteCommunicationServerLLGS(const lldb::PlatformSP& platform_sp, MainLoop &mainloop);
 
     virtual
     ~GDBRemoteCommunicationServerLLGS();
@@ -111,9 +113,13 @@ public:
     void
     DidExec (NativeProcessProtocol *process) override;
 
+    Error
+    InitializeConnection (std::unique_ptr<Connection> &&connection);
+
 protected:
     lldb::PlatformSP m_platform_sp;
-    lldb::thread_t m_async_thread;
+    MainLoop &m_mainloop;
+    MainLoop::ReadHandleUP m_read_handle_up;
     lldb::tid_t m_current_tid;
     lldb::tid_t m_continue_tid;
     Mutex m_debugged_process_mutex;
@@ -124,6 +130,7 @@ protected:
     Mutex m_saved_registers_mutex;
     std::unordered_map<uint32_t, lldb::DataBufferSP> m_saved_registers_map;
     uint32_t m_next_saved_registers_id;
+    bool m_handshake_completed : 1;
 
     PacketResult
     SendONotification (const char *buffer, uint32_t len);
@@ -295,6 +302,9 @@ private:
     void
     RegisterPacketHandlers ();
 
+    void
+    DataAvailableCallback ();
+
     //------------------------------------------------------------------
     // For GDBRemoteCommunicationServerLLGS only
     //------------------------------------------------------------------

Modified: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp?rev=242018&r1=242017&r2=242018&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp (original)
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp Mon Jul 13 05:44:55 2015
@@ -109,16 +109,21 @@ signal_handler(int signo)
     case SIGPIPE:
         g_sigpipe_received = 1;
         break;
-    case SIGHUP:
-        ++g_sighup_received_count;
-
-        // For now, swallow SIGHUP.
-        if (log)
-            log->Printf ("lldb-server:%s swallowing SIGHUP (receive count=%d)", __FUNCTION__, g_sighup_received_count);
-        signal (SIGHUP, signal_handler);
-        break;
     }
 }
+
+static void
+sighup_handler(MainLoopBase &mainloop)
+{
+    ++g_sighup_received_count;
+
+    Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
+    if (log)
+        log->Printf ("lldb-server:%s swallowing SIGHUP (receive count=%d)", __FUNCTION__, g_sighup_received_count);
+
+    if (g_sighup_received_count >= 2)
+        mainloop.RequestTermination();
+}
 #endif // #ifndef _WIN32
 
 static void
@@ -338,7 +343,7 @@ writePortToPipe(int unnamed_pipe_fd, con
 }
 
 void
-ConnectToRemote(GDBRemoteCommunicationServerLLGS &gdb_server,
+ConnectToRemote(MainLoop &mainloop, GDBRemoteCommunicationServerLLGS &gdb_server,
         bool reverse_connect, const char *const host_and_port,
         const char *const progname, const char *const subcommand,
         const char *const named_pipe_path, int unnamed_pipe_fd)
@@ -372,6 +377,8 @@ ConnectToRemote(GDBRemoteCommunicationSe
             exit (1);
         }
 
+        std::unique_ptr<ConnectionFileDescriptor> connection_up;
+
         if (reverse_connect)
         {
             // llgs will connect to the gdb-remote client.
@@ -388,8 +395,7 @@ ConnectToRemote(GDBRemoteCommunicationSe
             snprintf(connection_url, sizeof(connection_url), "connect://%s", final_host_and_port.c_str ());
 
             // Create the connection.
-            std::unique_ptr<ConnectionFileDescriptor> connection_up (new ConnectionFileDescriptor ());
-            connection_up.reset (new ConnectionFileDescriptor ());
+            connection_up.reset(new ConnectionFileDescriptor);
             auto connection_result = connection_up->Connect (connection_url, &error);
             if (connection_result != eConnectionStatusSuccess)
             {
@@ -401,10 +407,6 @@ ConnectToRemote(GDBRemoteCommunicationSe
                 fprintf (stderr, "error: failed to connect to client at '%s': %s", connection_url, error.AsCString ());
                 exit (-1);
             }
-
-            // We're connected.
-            printf ("Connection established.\n");
-            gdb_server.SetConnection (connection_up.release());
         }
         else
         {
@@ -461,10 +463,7 @@ ConnectToRemote(GDBRemoteCommunicationSe
 
             // Ensure we connected.
             if (s_listen_connection_up)
-            {
-                printf ("Connection established '%s'\n", s_listen_connection_up->GetURI().c_str());
-                gdb_server.SetConnection (s_listen_connection_up.release());
-            }
+                connection_up = std::move(s_listen_connection_up);
             else
             {
                 fprintf (stderr, "failed to connect to '%s': %s\n", final_host_and_port.c_str (), error.AsCString ());
@@ -472,46 +471,13 @@ ConnectToRemote(GDBRemoteCommunicationSe
                 exit (1);
             }
         }
-    }
-
-    if (gdb_server.IsConnected())
-    {
-        // After we connected, we need to get an initial ack from...
-        if (gdb_server.HandshakeWithClient(&error))
+        error = gdb_server.InitializeConnection (std::move(connection_up));
+        if (error.Fail())
         {
-            // We'll use a half a second timeout interval so that an exit conditions can
-            // be checked that often.
-            const uint32_t TIMEOUT_USEC = 500000;
-
-            bool interrupt = false;
-            bool done = false;
-            while (!interrupt && !done && (g_sighup_received_count < 2))
-            {
-                const GDBRemoteCommunication::PacketResult result = gdb_server.GetPacketAndSendResponse (TIMEOUT_USEC, error, interrupt, done);
-                if ((result != GDBRemoteCommunication::PacketResult::Success) &&
-                    (result != GDBRemoteCommunication::PacketResult::ErrorReplyTimeout))
-                {
-                    // We're bailing out - we only support successful handling and timeouts.
-                    fprintf(stderr, "leaving packet loop due to PacketResult %d\n", result);
-                    break;
-                }
-            }
-
-            if (error.Fail())
-            {
-                fprintf(stderr, "error: %s\n", error.AsCString());
-            }
-        }
-        else
-        {
-            fprintf(stderr, "error: handshake with client failed\n");
+            fprintf(stderr, "Failed to initialize connection: %s\n", error.AsCString());
+            exit(-1);
         }
-    }
-    else
-    {
-        fprintf (stderr, "no connection information provided, unable to run\n");
-        display_usage (progname, subcommand);
-        exit (1);
+        printf ("Connection established.\n");
     }
 }
 
@@ -521,10 +487,12 @@ ConnectToRemote(GDBRemoteCommunicationSe
 int
 main_gdbserver (int argc, char *argv[])
 {
+    Error error;
+    MainLoop mainloop;
 #ifndef _WIN32
     // Setup signal handlers first thing.
     signal (SIGPIPE, signal_handler);
-    signal (SIGHUP, signal_handler);
+    MainLoop::SignalHandleUP sighup_handle = mainloop.RegisterSignal(SIGHUP, sighup_handler, error);
 #endif
 #ifdef __linux__
     // Block delivery of SIGCHLD on linux. NativeProcessLinux will read it using signalfd.
@@ -539,7 +507,6 @@ main_gdbserver (int argc, char *argv[])
     argc--;
     argv++;
     int long_option_index = 0;
-    Error error;
     int ch;
     std::string platform_name;
     std::string attach_target;
@@ -670,7 +637,7 @@ main_gdbserver (int argc, char *argv[])
     // Setup the platform that GDBRemoteCommunicationServerLLGS will use.
     lldb::PlatformSP platform_sp = setup_platform (platform_name);
 
-    GDBRemoteCommunicationServerLLGS gdb_server (platform_sp);
+    GDBRemoteCommunicationServerLLGS gdb_server (platform_sp, mainloop);
 
     const char *const host_and_port = argv[0];
     argc -= 1;
@@ -688,10 +655,19 @@ main_gdbserver (int argc, char *argv[])
     // Print version info.
     printf("%s-%s", LLGS_PROGRAM_NAME, LLGS_VERSION_STR);
 
-    ConnectToRemote(gdb_server, reverse_connect,
+    ConnectToRemote(mainloop, gdb_server, reverse_connect,
                     host_and_port, progname, subcommand,
                     named_pipe_path.c_str(), unnamed_pipe_fd);
 
+
+    if (! gdb_server.IsConnected())
+    {
+        fprintf (stderr, "no connection information provided, unable to run\n");
+        display_usage (progname, subcommand);
+        return 1;
+    }
+
+    mainloop.Run();
     fprintf(stderr, "lldb-server exiting...\n");
 
     return 0;

Modified: lldb/trunk/tools/lldb-server/lldb-platform.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-server/lldb-platform.cpp?rev=242018&r1=242017&r2=242018&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-server/lldb-platform.cpp (original)
+++ lldb/trunk/tools/lldb-server/lldb-platform.cpp Mon Jul 13 05:44:55 2015
@@ -362,7 +362,7 @@ main_platform (int argc, char *argv[])
         if (platform.IsConnected())
         {
             // After we connected, we need to get an initial ack from...
-            if (platform.HandshakeWithClient(&error))
+            if (platform.HandshakeWithClient())
             {
                 bool interrupt = false;
                 bool done = false;





More information about the lldb-commits mailing list