[lldb] Use WSAEventSelect for MainLoop polling on windows

[labath]

Summary

This patch switches the MainLoop class to use the WSAEventSelect
mechanism to wait for multiple sockets to become readable. The
motivation for doing that is that this allows us to wait for other kinds
of events as well (as long as they can be converted to WSAEvents). This
will allow us to avoid (abstract away) pipe-based multiplexing
mechanisms in the generic code, since pipes cannot be combined with
sockets on windows.

Since the windows implementation will now look completely different than
the posix (file descriptor-based) implementations, I have split the
MainLoop class into two (MainLoopPosix and MainLoopWindows), with the
common code going into MainLoopBase.

[jingham]

This is a WIP, presumably in the final version there won't be prominent #ifdef _WIN32 in a file in the "Host/common" directory.

[mgorny]

Only guessing what various WSA functions do, it seems to all make sense to me. Just don't forget to clang-format ;-).

[labath]

In D131159#3699826, @jingham wrote:

This is a WIP, presumably in the final version there won't be prominent #ifdef _WIN32 in a file in the "Host/common" directory.

Yeah, that's the main reason for the WIPness. I still have to figure out how to factor this (there's a fair amount of code that can still be shared between the two implementations, but there's also an (increasing) portion that cannot.

[labath]

non-wip version

[mgorny]

Thanks for doing this. The common and POSIX parts LGTM. Just one minor suggestion since you've touched that line.

lldb/source/Host/posix/MainLoopPosix.cpp

124

[mstorsjo]

I don't really have experience with this API - is this pattern (creating and closing event objects for each poll run) the common way of using this API? Is there any potential performance risk compared with keeping event objects around (if that's possible?).

Can @alvinhochun take the patch for a spin? I think you've got more realistic usecases of lldb to try it out in. (Do you want me to make a build with the patch for you to try out?) @labath What's the best way to exercise this code in actual use of lldb?

[labath]

In D131159#3707741, @mstorsjo wrote:

I don't really have experience with this API - is this pattern (creating and closing event objects for each poll run) the common way of using this API? Is there any potential performance risk compared with keeping event objects around (if that's possible?).

Well.. I can't exactly say I'm familiar with the APIs either, but I suspect this is not the "common" way of using them. It is, however, the closest I could get to the purely level-triggered behavior that one gets with select/poll on unix. Normally (with event reuse), windows requires that one calls the "reset" function (e.g. recv) at least once (the call itself is sufficient, it does not have to actually read all pending data) before the event can be triggered again. I think possible that nothing depends on this behavior (or could be fixed to not depend on it) -- after all you're usually doing this because you actually want to read from the socket. However, we did have one unit test which relied on this. That was mostly because it was being lazy (testing other things), but I wanted to start out with something safe-ish.

As for performance, it's definitely slower than not creating the events every time around, but I doubt that difference would be noticeable. However, now that I think about it, I think the repeating the call to WSAEventSelect should be sufficient to get the level-triggered behavior, even if it's just reusing the same event object. Let me try how that works.

Can @alvinhochun take the patch for a spin? I think you've got more realistic usecases of lldb to try it out in. (Do you want me to make a build with the patch for you to try out?) @labath What's the best way to exercise this code in actual use of lldb?

I would appreciate that. I have tried to build it and run the test suite, but I didn't have a clean baseline, so I can't really say whether it works for everything. The main user of this code is the gdb-remote communication code, so debugging something via lldb-server (or running the lldb-server test suite) should be a good indicator of whether it works.

[mstorsjo]

In D131159#3709965, @labath wrote:

Can @alvinhochun take the patch for a spin? I think you've got more realistic usecases of lldb to try it out in. (Do you want me to make a build with the patch for you to try out?) @labath What's the best way to exercise this code in actual use of lldb?

I would appreciate that. I have tried to build it and run the test suite, but I didn't have a clean baseline, so I can't really say whether it works for everything. The main user of this code is the gdb-remote communication code, so debugging something via lldb-server (or running the lldb-server test suite) should be a good indicator of whether it works.

I presume this is covered by running ninja check-lldb, or should I flip the switch to prefer lldb-server on Windows before doing that? (I guess I can do both.) I've got an environment where check-lldb mostly passes, I can check that it doesn't regress it.

[mstorsjo]

In D131159#3711024, @mstorsjo wrote:

In D131159#3709965, @labath wrote:

Can @alvinhochun take the patch for a spin? I think you've got more realistic usecases of lldb to try it out in. (Do you want me to make a build with the patch for you to try out?) @labath What's the best way to exercise this code in actual use of lldb?

I would appreciate that. I have tried to build it and run the test suite, but I didn't have a clean baseline, so I can't really say whether it works for everything. The main user of this code is the gdb-remote communication code, so debugging something via lldb-server (or running the lldb-server test suite) should be a good indicator of whether it works.

I presume this is covered by running ninja check-lldb, or should I flip the switch to prefer lldb-server on Windows before doing that? (I guess I can do both.) I've got an environment where check-lldb mostly passes, I can check that it doesn't regress it.

I tested out this patch with running ninja check-lldb. Out of the box, this patch doesn't regress anything - the tests still pass. (In my environment, there's a bunch of occasional stray failures already before, but by rerunning the tests a couple times, I can get a clean run.)

If I edit ShouldUseLLDBServer() in lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp to always return true (before applying this patch), most tests still pass, but I have 11 tests that hang so that I end up having to kill lldb-server. With this patch on top, I still get the same set of 11 hanging tests, so I think that means that this patch doesn't regress anything with respect to that.

[alvinhochun]

I gave this a spin with LLDB_USE_LLDB_SERVER=1, but there looks to be some pre-existing issues with the lldb-server implementation so I can't really test much normally using builds of Krita:

• Manually set breakpoints don't work (they stay unresolved?)
• Debugger apparently breaks on every first chance exceptions (running ASAN builds stops too often)
• Backtraces have no symbol names and seems to show bogus addresses

Other than that, it runs the debuggee and I can continue on exception stops. That's as much as I can tell.

[labath]

Switch to the implementation which uses persistent WSAEvents for
listening. I have no idea whether this makes a difference in practice,
but it /feels/ more efficient.

[labath]

Thank you, Martin and Alvin for trying this out. Setting LLDB_USE_LLDB_SERVER definitely increases the number of tests that use lldb-server, but I wouldn't really say it's necessary, as I would expect that a change like this either works, or fails spectacularly.

Can I assume that you are ok with checking this in?

In D131159#3709965, @labath wrote:

As for performance, it's definitely slower than not creating the events every time around, but I doubt that difference would be noticeable. However, now that I think about it, I think the repeating the call to WSAEventSelect should be sufficient to get the level-triggered behavior, even if it's just reusing the same event object. Let me try how that works.

I have implemented this now.

[mstorsjo]

FYI, this patch added some new compilation warnings:

[4055/7050] Building CXX object tools/lldb/source/Host/CMakeFiles/lldbHost.dir/windows/MainLoopWindows.cpp.obj
llvm-project/lldb/source/Host/windows/MainLoopWindows.cpp:28:9: warning: unused variable 'result' [-Wunused-variable]
    int result = WSAEventSelect(fd, info.event, FD_READ | FD_ACCEPT | FD_CLOSE);
        ^
llvm-project/lldb/source/Host/windows/MainLoopWindows.cpp:38:9: warning: unused variable 'result' [-Wunused-variable]
    int result = WSAEventSelect(fd.first, WSA_INVALID_EVENT, 0);
        ^
llvm-project/lldb/source/Host/windows/MainLoopWindows.cpp:86:8: warning: unused variable 'result' [-Wunused-variable]
  BOOL result = WSACloseEvent(it->second.event);
       ^
3 warnings generated.

(They're only used in asserts.) I guess we should add void casts to mark the variables as used outside of asserts.

[mgorny]

In D131159#3742235, @mstorsjo wrote:

(They're only used in asserts.) I guess we should add void casts to mark the variables as used outside of asserts.

Actually, there's a dedicated UNUSED_IF_ASSERT_DISABLED macro for that. Please use it instead.

[labath]

In D131159#3742364, @mgorny wrote:

In D131159#3742235, @mstorsjo wrote:

(They're only used in asserts.) I guess we should add void casts to mark the variables as used outside of asserts.

Actually, there's a dedicated UNUSED_IF_ASSERT_DISABLED macro for that. Please use it instead.

One of these days, I'm going to have to delete it. If void casts are good enough for the rest of llvm, I don't see why should we be any different.

[mstorsjo]

In D131159#3742370, @labath wrote:

In D131159#3742364, @mgorny wrote:

In D131159#3742235, @mstorsjo wrote:

(They're only used in asserts.) I guess we should add void casts to mark the variables as used outside of asserts.

Actually, there's a dedicated UNUSED_IF_ASSERT_DISABLED macro for that. Please use it instead.

One of these days, I'm going to have to delete it. If void casts are good enough for the rest of llvm, I don't see why should we be any different.

Ok, so is this discussion settled and I can go ahead and use void casts for these trivial fixups? (I usually just push such patches directly without passing via review.)

[labath]

In D131159#3743497, @mstorsjo wrote:

In D131159#3742370, @labath wrote:

In D131159#3742364, @mgorny wrote:

In D131159#3742235, @mstorsjo wrote:

(They're only used in asserts.) I guess we should add void casts to mark the variables as used outside of asserts.

Actually, there's a dedicated UNUSED_IF_ASSERT_DISABLED macro for that. Please use it instead.

One of these days, I'm going to have to delete it. If void casts are good enough for the rest of llvm, I don't see why should we be any different.

Ok, so is this discussion settled and I can go ahead and use void casts for these trivial fixups? (I usually just push such patches directly without passing via review.)

I'm might not call it "settled", but I don't think anybody will be too upset regardless of that you do. You won't be the first or the last one to push such patches, and that's the reason why I think that the UNUSED_IF_ASSERT_DISABLED macro is doomed to fail (or at least be used inconsistently).

lldb/include/lldb/Host/MainLoop.h

 //===-- MainLoop.h ----------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_HOST_MAINLOOP_H
 #define LLDB_HOST_MAINLOOP_H
 
-#include "lldb/Host/Config.h"
-#include "lldb/Host/MainLoopBase.h"
-#include "llvm/ADT/DenseMap.h"
-#include <csignal>
-#include <list>
-#include <vector>
-
-#if !HAVE_PPOLL && !HAVE_SYS_EVENT_H && !defined(__ANDROID__)
-#define SIGNAL_POLLING_UNSUPPORTED 1
-#endif
-
+#ifdef _WIN32
+#include "lldb/Host/windows/MainLoopWindows.h"
 namespace lldb_private {
-
-// Implementation of the MainLoopBase class. It can monitor file descriptors
-// for readability using ppoll, kqueue, poll or WSAPoll. On Windows it only
-// supports polling sockets, and will not work on generic file handles or
-// pipes. On systems without kqueue or ppoll handling singnals is not
-// supported. 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 MainLoop : public MainLoopBase {
-private:
-  class SignalHandle;
-
-public:
-  typedef std::unique_ptr<SignalHandle> SignalHandleUP;
-
-  MainLoop();
-  ~MainLoop() override;
-
-  ReadHandleUP RegisterReadObject(const lldb::IOObjectSP &object_sp,
-                                  const Callback &callback,
-                                  Status &error) override;
-
-  // Listening for signals from multiple MainLoop 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 mean 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,
-                                Status &error);
-
-  // Add a pending callback that will be executed once after all the pending
-  // events are processed. The callback will be executed even if termination
-  // was requested.
-  void AddPendingCallback(const Callback &callback) override;
-
-  Status 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(IOObject::WaitableHandle handle) override;
-
-  void UnregisterSignal(int signo, std::list<Callback>::iterator callback_it);
-
-private:
-  void ProcessReadObject(IOObject::WaitableHandle handle);
-  void ProcessSignal(int signo);
-
-  class SignalHandle {
-  public:
-    ~SignalHandle() { m_mainloop.UnregisterSignal(m_signo, m_callback_it); }
-
-  private:
-    SignalHandle(MainLoop &mainloop, int signo,
-                 std::list<Callback>::iterator callback_it)
-        : m_mainloop(mainloop), m_signo(signo), m_callback_it(callback_it) {}
-
-    MainLoop &m_mainloop;
-    int m_signo;
-    std::list<Callback>::iterator m_callback_it;
-
-    friend class MainLoop;
-    SignalHandle(const SignalHandle &) = delete;
-    const SignalHandle &operator=(const SignalHandle &) = delete;
-  };
-
-  struct SignalInfo {
-    std::list<Callback> callbacks;
-#ifndef SIGNAL_POLLING_UNSUPPORTED
-    struct sigaction old_action;
-#endif
-    bool was_blocked : 1;
-  };
-  class RunImpl;
-
-  llvm::DenseMap<IOObject::WaitableHandle, Callback> m_read_fds;
-  llvm::DenseMap<int, SignalInfo> m_signals;
-  std::vector<Callback> m_pending_callbacks;
-#if HAVE_SYS_EVENT_H
-  int m_kqueue;
+using MainLoop = MainLoopWindows;
+}
+#else
+#include "lldb/Host/posix/MainLoopPosix.h"
+namespace lldb_private {
+using MainLoop = MainLoopPosix;
+}
 #endif
-  bool m_terminate_request : 1;
-};
-
-} // namespace lldb_private
 
 #endif // LLDB_HOST_MAINLOOP_H

lldb/include/lldb/Host/MainLoopBase.h

 //===-- MainLoopBase.h ------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_HOST_MAINLOOPBASE_H
 #define LLDB_HOST_MAINLOOPBASE_H
 
 #include "lldb/Utility/IOObject.h"
 #include "lldb/Utility/Status.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <functional>
 
 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.
+// 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.
 class MainLoopBase {
 private:
   class ReadHandle;
 
 public:
-  MainLoopBase() = default;
-  virtual ~MainLoopBase() = default;
+  MainLoopBase() : m_terminate_request(false) {}
+  virtual ~MainLoopBase() { assert(m_read_fds.size() == 0); }
 
   typedef std::unique_ptr<ReadHandle> ReadHandleUP;
 
   typedef std::function<void(MainLoopBase &)> Callback;
 
   virtual ReadHandleUP RegisterReadObject(const lldb::IOObjectSP &object_sp,
                                           const Callback &callback,
-                                          Status &error) {
-    llvm_unreachable("Not implemented");
-  }
+                                          Status &error);
 
   // Add a pending callback that will be executed once after all the pending
   // events are processed. The callback will be executed even if termination
   // was requested.
   virtual void AddPendingCallback(const Callback &callback) {
-    llvm_unreachable("Not implemented");
+    m_pending_callbacks.push_back(callback);
   }
 
   // Waits for registered events and invoke the proper callbacks. Returns when
   // all callbacks deregister themselves or when someone requests termination.
   virtual Status Run() { llvm_unreachable("Not implemented"); }
 
-  // Requests the exit of the Run() function.
-  virtual void RequestTermination() { llvm_unreachable("Not implemented"); }
+  // This should only be performed from a callback. Do not attempt to terminate
+  // the processing from another thread.
+  virtual void RequestTermination() { m_terminate_request = true; }
 
 protected:
   ReadHandleUP CreateReadHandle(const lldb::IOObjectSP &object_sp) {
     return ReadHandleUP(new ReadHandle(*this, object_sp->GetWaitableHandle()));
   }
 
-  virtual void UnregisterReadObject(IOObject::WaitableHandle handle) {
-    llvm_unreachable("Not implemented");
-  }
+  virtual void UnregisterReadObject(IOObject::WaitableHandle handle);
+
+  void ProcessReadObject(IOObject::WaitableHandle handle);
+  void ProcessPendingCallbacks();
+
+  llvm::DenseMap<IOObject::WaitableHandle, Callback> m_read_fds;
+  std::vector<Callback> m_pending_callbacks;
+  bool m_terminate_request : 1;
 
 private:
   class ReadHandle {
   public:
     ~ReadHandle() { m_mainloop.UnregisterReadObject(m_handle); }
 
   private:
     ReadHandle(MainLoopBase &mainloop, IOObject::WaitableHandle handle)

lldb/include/lldb/Host/posix/MainLoopPosix.h

+//===-- MainLoopPosix.h -----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_HOST_POSIX_MAINLOOPPOSIX_H
+#define LLDB_HOST_POSIX_MAINLOOPPOSIX_H
+
+#include "lldb/Host/Config.h"
+#include "lldb/Host/MainLoopBase.h"
+#include "llvm/ADT/DenseMap.h"
+#include <csignal>
+#include <list>
+#include <vector>
+
+namespace lldb_private {
+
+// Implementation of the MainLoopBase class. It can monitor file descriptors for
+// readability using ppoll, kqueue, or pselect. In addition to the common base,
+// this class provides the ability to invoke a given handler when a signal is
+// received.
+class MainLoopPosix : public MainLoopBase {
+private:
+  class SignalHandle;
+
+public:
+  typedef std::unique_ptr<SignalHandle> SignalHandleUP;
+
+  MainLoopPosix();
+  ~MainLoopPosix() override;
+
+  // Listening for signals from multiple MainLoop 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 mean 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,
+                                Status &error);
+
+  Status Run() override;
+
+protected:
+  void UnregisterSignal(int signo, std::list<Callback>::iterator callback_it);
+
+private:
+  void ProcessSignal(int signo);
+
+  class SignalHandle {
+  public:
+    ~SignalHandle() { m_mainloop.UnregisterSignal(m_signo, m_callback_it); }
+
+  private:
+    SignalHandle(MainLoopPosix &mainloop, int signo,
+                 std::list<Callback>::iterator callback_it)
+        : m_mainloop(mainloop), m_signo(signo), m_callback_it(callback_it) {}
+
+    MainLoopPosix &m_mainloop;
+    int m_signo;
+    std::list<Callback>::iterator m_callback_it;
+
+    friend class MainLoopPosix;
+    SignalHandle(const SignalHandle &) = delete;
+    const SignalHandle &operator=(const SignalHandle &) = delete;
+  };
+
+  struct SignalInfo {
+    std::list<Callback> callbacks;
+    struct sigaction old_action;
+    bool was_blocked : 1;
+  };
+  class RunImpl;
+
+  llvm::DenseMap<int, SignalInfo> m_signals;
+#if HAVE_SYS_EVENT_H
+  int m_kqueue;
+#endif
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_HOST_POSIX_MAINLOOPPOSIX_H

lldb/include/lldb/Host/windows/MainLoopWindows.h

+//===-- MainLoopWindows.h ---------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_HOST_WINDOWS_MAINLOOPWINDOWS_H
+#define LLDB_HOST_WINDOWS_MAINLOOPWINDOWS_H
+
+#include "lldb/Host/Config.h"
+#include "lldb/Host/MainLoopBase.h"
+#include <csignal>
+#include <list>
+#include <vector>
+
+namespace lldb_private {
+
+// Windows-specific implementation of the MainLoopBase class. It can monitor
+// socket descriptors for readability using WSAEventSelect. Non-socket file
+// descriptors are not supported.
+class MainLoopWindows : public MainLoopBase {
+public:
+  ReadHandleUP RegisterReadObject(const lldb::IOObjectSP &object_sp,
+                                  const Callback &callback,
+                                  Status &error) override;
+
+  Status Run() override;
+
+private:
+  llvm::Expected<size_t> Poll();
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_HOST_WINDOWS_MAINLOOPWINDOWS_H

lldb/source/Host/CMakeLists.txt

add_host_subdirectory(common
   common/GetOptInc.cpp
   common/Host.cpp
   common/HostInfoBase.cpp
   common/HostNativeThreadBase.cpp
   common/HostProcess.cpp
   common/HostThread.cpp
   common/LockFileBase.cpp
   common/LZMA.cpp
-  common/MainLoop.cpp
+  common/MainLoopBase.cpp
   common/MonitoringProcessLauncher.cpp
   common/NativeProcessProtocol.cpp
   common/NativeRegisterContext.cpp
   common/NativeThreadProtocol.cpp
   common/NativeWatchpointList.cpp
   common/OptionParser.cpp
   common/PipeBase.cpp
   common/ProcessLaunchInfo.cpp
if (CMAKE_SYSTEM_NAME MATCHES "Windows")
   add_host_subdirectory(windows
     windows/ConnectionGenericFileWindows.cpp
     windows/FileSystem.cpp
     windows/Host.cpp
     windows/HostInfoWindows.cpp
     windows/HostProcessWindows.cpp
     windows/HostThreadWindows.cpp
     windows/LockFileWindows.cpp
+    windows/MainLoopWindows.cpp
     windows/PipeWindows.cpp
     windows/ProcessLauncherWindows.cpp
     windows/ProcessRunLock.cpp
     )
 else()
   add_host_subdirectory(posix
     posix/DomainSocket.cpp
     posix/FileSystemPosix.cpp
     posix/HostInfoPosix.cpp
     posix/HostProcessPosix.cpp
     posix/HostThreadPosix.cpp
     posix/LockFilePosix.cpp
+    posix/MainLoopPosix.cpp
     posix/PipePosix.cpp
     posix/ProcessLauncherPosixFork.cpp
     )
 
   if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
     add_subdirectory(macosx/objcxx)
     set(LLDBObjCLibs lldbHostMacOSXObjCXX)
     add_host_subdirectory(macosx

lldb/source/Host/common/MainLoopBase.cpp

+//===-- MainLoopBase.cpp --------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/MainLoopBase.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+MainLoopBase::ReadHandleUP
+MainLoopBase::RegisterReadObject(const IOObjectSP &object_sp,
+                                 const Callback &callback, Status &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);
+}
+
+void MainLoopBase::UnregisterReadObject(IOObject::WaitableHandle handle) {
+  bool erased = m_read_fds.erase(handle);
+  UNUSED_IF_ASSERT_DISABLED(erased);
+  assert(erased);
+}
+
+void MainLoopBase::ProcessReadObject(IOObject::WaitableHandle handle) {
+  auto it = m_read_fds.find(handle);
+  if (it != m_read_fds.end())
+    it->second(*this); // Do the work
+}
+
+void MainLoopBase::ProcessPendingCallbacks() {
+  for (const Callback &callback : m_pending_callbacks)
+    callback(*this);
+  m_pending_callbacks.clear();
+}

lldb/source/Host/posix/MainLoopPosix.cpp

-//===-- MainLoop.cpp ------------------------------------------------------===//
+//===-- MainLoopPosix.cpp -------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/Config/llvm-config.h"
+#include "lldb/Host/posix/MainLoopPosix.h"
 #include "lldb/Host/Config.h"
-
-#include "lldb/Host/MainLoop.h"
 #include "lldb/Host/PosixApi.h"
 #include "lldb/Utility/Status.h"
+#include "llvm/Config/llvm-config.h"
 #include <algorithm>
 #include <cassert>
 #include <cerrno>
 #include <csignal>
 #include <ctime>
 #include <vector>
 
 // Multiplexing is implemented using kqueue on systems that support it (BSD
 // variants including OSX). On linux we use ppoll, while android uses pselect
 // (ppoll is present but not implemented properly). On windows we use WSApoll
 // (which does not support signals).
 
 #if HAVE_SYS_EVENT_H
 #include <sys/event.h>
-#elif defined(_WIN32)
-#include <winsock2.h>
 #elif defined(__ANDROID__)
 #include <sys/syscall.h>
 #else
 #include <poll.h>
 #endif
 
-#ifdef _WIN32
-#define POLL WSAPoll
-#else
-#define POLL poll
-#endif
-
-#if SIGNAL_POLLING_UNSUPPORTED
-#ifdef _WIN32
-typedef int sigset_t;
-typedef int siginfo_t;
-#endif
-
-int ppoll(struct pollfd *fds, size_t nfds, const struct timespec *timeout_ts,
-          const sigset_t *) {
-  int timeout =
-      (timeout_ts == nullptr)
-          ? -1
-          : (timeout_ts->tv_sec * 1000 + timeout_ts->tv_nsec / 1000000);
-  return POLL(fds, nfds, timeout);
-}
-
-#endif
-
 using namespace lldb;
 using namespace lldb_private;
 
 static sig_atomic_t g_signal_flags[NSIG];
 
-#ifndef SIGNAL_POLLING_UNSUPPORTED
 static void SignalHandler(int signo, siginfo_t *info, void *) {
   assert(signo < NSIG);
   g_signal_flags[signo] = 1;
 }
-#endif
 
-class MainLoop::RunImpl {
+class MainLoopPosix::RunImpl {
 public:
-  RunImpl(MainLoop &loop);
+  RunImpl(MainLoopPosix &loop);
   ~RunImpl() = default;
 
   Status Poll();
   void ProcessEvents();
 
 private:
-  MainLoop &loop;
+  MainLoopPosix &loop;
 
 #if HAVE_SYS_EVENT_H
   std::vector<struct kevent> in_events;
   struct kevent out_events[4];
   int num_events = -1;
 
 #else
 #ifdef __ANDROID__
   fd_set read_fd_set;
 #else
   std::vector<struct pollfd> read_fds;
 #endif
 
   sigset_t get_sigmask();
 #endif
 };
 
 #if HAVE_SYS_EVENT_H
-MainLoop::RunImpl::RunImpl(MainLoop &loop) : loop(loop) {
+MainLoopPosix::RunImpl::RunImpl(MainLoopPosix &loop) : loop(loop) {
   in_events.reserve(loop.m_read_fds.size());
 }
 
-Status MainLoop::RunImpl::Poll() {
+Status MainLoopPosix::RunImpl::Poll() {
   in_events.resize(loop.m_read_fds.size());
   unsigned i = 0;
   for (auto &fd : loop.m_read_fds)
     EV_SET(&in_events[i++], fd.first, EVFILT_READ, EV_ADD, 0, 0, 0);
 
   num_events = kevent(loop.m_kqueue, in_events.data(), in_events.size(),
                       out_events, llvm::array_lengthof(out_events), nullptr);
 
   if (num_events < 0) {
     if (errno == EINTR) {
       // in case of EINTR, let the main loop run one iteration
       // we need to zero num_events to avoid assertions failing
       num_events = 0;
     } else
       return Status(errno, eErrorTypePOSIX);
   }
   return Status();
 }
 
-void MainLoop::RunImpl::ProcessEvents() {
+void MainLoopPosix::RunImpl::ProcessEvents() {
   assert(num_events >= 0);
   for (int i = 0; i < num_events; ++i) {
     if (loop.m_terminate_request)
       return;
     switch (out_events[i].filter) {
     case EVFILT_READ:
       loop.ProcessReadObject(out_events[i].ident);
       break;
     case EVFILT_SIGNAL:
       loop.ProcessSignal(out_events[i].ident);
       break;
     default:
       llvm_unreachable("Unknown event");
     }
   }
 }
 #else
-MainLoop::RunImpl::RunImpl(MainLoop &loop) : loop(loop) {
+MainLoopPosix::RunImpl::RunImpl(MainLoopPosix &loop) : loop(loop) {
 #ifndef __ANDROID__
   read_fds.reserve(loop.m_read_fds.size());
 #endif
 }
 
-sigset_t MainLoop::RunImpl::get_sigmask() {
+sigset_t MainLoopPosix::RunImpl::get_sigmask() {
   sigset_t sigmask;
-#if defined(_WIN32)
-  sigmask = 0;
-#elif SIGNAL_POLLING_UNSUPPORTED
-  sigemptyset(&sigmask);
-#else
   int ret = pthread_sigmask(SIG_SETMASK, nullptr, &sigmask);
   assert(ret == 0);
-  (void) ret;
+  (void)ret;
 
   for (const auto &sig : loop.m_signals)
     sigdelset(&sigmask, sig.first);
-#endif
   return sigmask;
 }
 
 #ifdef __ANDROID__
-Status MainLoop::RunImpl::Poll() {
+Status MainLoopPosix::RunImpl::Poll() {
   // ppoll(2) is not supported on older all android versions. Also, older
   // versions android (API <= 19) implemented pselect in a non-atomic way, as a
   // combination of pthread_sigmask and select. This is not sufficient for us,
   // as we rely on the atomicity to correctly implement signal polling, so we
   // call the underlying syscall ourselves.
 
   FD_ZERO(&read_fd_set);
   int nfds = 0;
Status MainLoopPosix::RunImpl::Poll() {
   if (syscall(__NR_pselect6, nfds, &read_fd_set, nullptr, nullptr, nullptr,
               &extra_data) == -1 &&
       errno != EINTR)
     return Status(errno, eErrorTypePOSIX);
 
   return Status();
 }
 #else
-Status MainLoop::RunImpl::Poll() {
+Status MainLoopPosix::RunImpl::Poll() {
   read_fds.clear();
 
   sigset_t sigmask = get_sigmask();
 
   for (const auto &fd : loop.m_read_fds) {
     struct pollfd pfd;
     pfd.fd = fd.first;
     pfd.events = POLLIN;
     pfd.revents = 0;
     read_fds.push_back(pfd);
   }
 
   if (ppoll(read_fds.data(), read_fds.size(), nullptr, &sigmask) == -1 &&
       errno != EINTR)
     return Status(errno, eErrorTypePOSIX);
 
   return Status();
 }
 #endif
 
-void MainLoop::RunImpl::ProcessEvents() {
+void MainLoopPosix::RunImpl::ProcessEvents() {
 #ifdef __ANDROID__
   // Collect first all readable file descriptors into a separate vector and
   // then iterate over it to invoke callbacks. Iterating directly over
   // loop.m_read_fds is not possible because the callbacks can modify the
   // container which could invalidate the iterator.
   std::vector<IOObject::WaitableHandle> fds;
   for (const auto &fd : loop.m_read_fds)
     if (FD_ISSET(fd.first, &read_fd_set))
for (const auto &signal : signals) {
     if (loop.m_terminate_request)
       return;
     g_signal_flags[signal] = 0;
     loop.ProcessSignal(signal);
   }
 }
 #endif
 
-MainLoop::MainLoop() : m_terminate_request(false) {
+MainLoopPosix::MainLoopPosix() {
 #if HAVE_SYS_EVENT_H
   m_kqueue = kqueue();
   assert(m_kqueue >= 0);
 #endif
 }
-MainLoop::~MainLoop() {
+
+MainLoopPosix::~MainLoopPosix() {
 #if HAVE_SYS_EVENT_H
   close(m_kqueue);
 #endif
-  assert(m_read_fds.size() == 0);
   assert(m_signals.size() == 0);
 }
 
-MainLoop::ReadHandleUP MainLoop::RegisterReadObject(const IOObjectSP &object_sp,
-                                                    const Callback &callback,
-                                                    Status &error) {
-#ifdef _WIN32
-  if (object_sp->GetFdType() != IOObject:: eFDTypeSocket) {
-    error.SetErrorString("MainLoop: non-socket types unsupported on Windows");
-    return nullptr;
-  }
-#endif
-  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.
-MainLoop::SignalHandleUP
-MainLoop::RegisterSignal(int signo, const Callback &callback, Status &error) {
-#ifdef SIGNAL_POLLING_UNSUPPORTED
-  error.SetErrorString("Signal polling is not supported on this platform.");
-  return nullptr;
-#else
+MainLoopPosix::SignalHandleUP
+MainLoopPosix::RegisterSignal(int signo, const Callback &callback,
+                              Status &error) {
   auto signal_it = m_signals.find(signo);
   if (signal_it != m_signals.end()) {
     auto callback_it = signal_it->second.callbacks.insert(
         signal_it->second.callbacks.end(), callback);
     return SignalHandleUP(new SignalHandle(*this, signo, callback_it));
   }
 
   SignalInfo info;
#endif
   ret = pthread_sigmask(HAVE_SYS_EVENT_H ? SIG_UNBLOCK : SIG_BLOCK,
                         &new_action.sa_mask, &old_set);
   assert(ret == 0 && "pthread_sigmask failed");
   info.was_blocked = sigismember(&old_set, signo);
   auto insert_ret = m_signals.insert({signo, info});
 
   return SignalHandleUP(new SignalHandle(
       *this, signo, insert_ret.first->second.callbacks.begin()));
-#endif
 }
 
-void MainLoop::AddPendingCallback(const Callback &callback) {
-  m_pending_callbacks.push_back(callback);
-}
-
-void MainLoop::UnregisterReadObject(IOObject::WaitableHandle handle) {
-  bool erased = m_read_fds.erase(handle);
-  UNUSED_IF_ASSERT_DISABLED(erased);
-  assert(erased);
-}
-
-void MainLoop::UnregisterSignal(int signo,
-                                std::list<Callback>::iterator callback_it) {
-#if SIGNAL_POLLING_UNSUPPORTED
-  Status("Signal polling is not supported on this platform.");
-#else
+void MainLoopPosix::UnregisterSignal(
+    int signo, std::list<Callback>::iterator callback_it) {
   auto it = m_signals.find(signo);
   assert(it != m_signals.end());
 
   it->second.callbacks.erase(callback_it);
   // Do not remove the signal handler unless all callbacks have been erased.
   if (!it->second.callbacks.empty())
     return;
 

 #if HAVE_SYS_EVENT_H
   struct kevent ev;
   EV_SET(&ev, signo, EVFILT_SIGNAL, EV_DELETE, 0, 0, 0);
   ret = kevent(m_kqueue, &ev, 1, nullptr, 0, nullptr);
   assert(ret == 0);
 #endif
 
   m_signals.erase(it);
-#endif
 }
 
-Status MainLoop::Run() {
+Status MainLoopPosix::Run() {
   m_terminate_request = false;
 
   Status error;
   RunImpl impl(*this);
 
   // run until termination or until we run out of things to listen to
   while (!m_terminate_request && (!m_read_fds.empty() || !m_signals.empty())) {
 
     error = impl.Poll();
     if (error.Fail())
       return error;
 
     impl.ProcessEvents();
 
-    for (const Callback &callback : m_pending_callbacks)
-      callback(*this);
-    m_pending_callbacks.clear();
+    ProcessPendingCallbacks();
   }
   return Status();
 }
 
-void MainLoop::ProcessSignal(int signo) {
+void MainLoopPosix::ProcessSignal(int signo) {
   auto it = m_signals.find(signo);
   if (it != m_signals.end()) {
     // The callback may actually register/unregister signal handlers,
     // so we need to create a copy first.
     llvm::SmallVector<Callback, 4> callbacks_to_run{
         it->second.callbacks.begin(), it->second.callbacks.end()};
     for (auto &x : callbacks_to_run)
       x(*this); // Do the work
   }
 }
-
-void MainLoop::ProcessReadObject(IOObject::WaitableHandle handle) {
-  auto it = m_read_fds.find(handle);
-  if (it != m_read_fds.end())
-    it->second(*this); // Do the work
-}

lldb/source/Host/windows/MainLoopWindows.cpp

+//===-- MainLoopWindows.cpp -----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/windows/MainLoopWindows.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/Config/llvm-config.h"
+#include <algorithm>
+#include <cassert>
+#include <cerrno>
+#include <csignal>
+#include <ctime>
+#include <vector>
+#include <winsock2.h>
+
+using namespace lldb;
+using namespace lldb_private;
+
+llvm::Expected<size_t> MainLoopWindows::Poll() {
+  std::vector<WSAEVENT> read_events;
+  read_events.reserve(m_read_fds.size());
+  for (auto &fd : m_read_fds) {
+    WSAEVENT event = WSACreateEvent();
+    assert(event != WSA_INVALID_EVENT);
+
+    int result =
+        WSAEventSelect(fd.first, event, FD_READ | FD_ACCEPT | FD_CLOSE);
+    assert(result == 0);
+
+    read_events.push_back(event);
+  }
+
+  DWORD result = WSAWaitForMultipleEvents(
+      read_events.size(), read_events.data(), FALSE, WSA_INFINITE, FALSE);
+
+  for (auto &fd : m_read_fds) {
+    int result = WSAEventSelect(fd.first, WSA_INVALID_EVENT, 0);
+    assert(result == 0);
+  }
+
+  for (auto &event : read_events) {
+    BOOL result = WSACloseEvent(event);
+    assert(result == TRUE);
+  }
+
+  if (result >= WSA_WAIT_EVENT_0 &&
+      result < WSA_WAIT_EVENT_0 + read_events.size())
+    return result - WSA_WAIT_EVENT_0;
+
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                 "WSAWaitForMultipleEvents failed");
+}
+
+MainLoopWindows::ReadHandleUP
+MainLoopWindows::RegisterReadObject(const IOObjectSP &object_sp,
+                                    const Callback &callback, Status &error) {
+  if (object_sp->GetFdType() != IOObject::eFDTypeSocket) {
+    error.SetErrorString(
+        "MainLoopWindows: non-socket types unsupported on Windows");
+    return nullptr;
+  }
+  return MainLoopBase::RegisterReadObject(object_sp, callback, error);
+}
+
+Status MainLoopWindows::Run() {
+  m_terminate_request = false;
+
+  Status error;
+
+  // run until termination or until we run out of things to listen to
+  while (!m_terminate_request && !m_read_fds.empty()) {
+
+    llvm::Expected<size_t> signaled_event = Poll();
+    if (!signaled_event)
+      return Status(signaled_event.takeError());
+
+    auto &fd_info = *std::next(m_read_fds.begin(), *signaled_event);
+
+    ProcessReadObject(fd_info.first);
+    ProcessPendingCallbacks();
+  }
+  return Status();
+}