This commit simplifies the Linux ProcessMonitor:
- DoOperation() and ServeOperation() no longer depend on file descriptors!
- remove EnableIPC() helper. ProcessMonitor will not have to run as a separate
process.
- added a semaphore to signal when an operation is complete (previously
signaling happened through a file descriptor)
- under heavy stress tests (i.e. running tests while compiling) we noticed that
the previous ProcessMonitor implementation would break because the read/write
calls were not error-checked, causing LLDB to hang.
Add lldb_private::Queue class:
- implemented on top of lldb's Mutex and Condition classes
- supports pop(), push() and empty() from multiple threads
http://llvm-reviews.chandlerc.com/D1682
Files:
include/lldb/Host/Queue.h
source/Plugins/Process/Linux/ProcessMonitor.cpp
source/Plugins/Process/Linux/ProcessMonitor.h
Index: include/lldb/Host/Queue.h
===================================================================
--- /dev/null
+++ include/lldb/Host/Queue.h
@@ -0,0 +1,80 @@
+//===-- Queue.h -------------------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+// A minimal thread-safe queue.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_PRIVATE_HOST_QUEUE
+#define LLDB_PRIVATE_HOST_QUEUE
+
+#include "lldb/Host/Mutex.h"
+#include "lldb/Host/Condition.h"
+#include <queue>
+
+namespace lldb_private {
+
+//-------------------------------------------
+/// @class Queue Queue.h "lldb/Host/Queue.h"
+/// @brief A thread-safe FIFO queue.
+//-------------------------------------------
+template<typename T>
+class Queue {
+public:
+ //-----------------------------------------------------------------------
+ /// Pops the element from the front of the queue.
+ ///
+ /// Removes the front-most element from the queue wrapped by this object,
+ /// and returns it by reference. If the queue is empty, this function
+ /// blocks until an element is pushed.
+ ///
+ /// @return
+ /// A reference to the element that was previously at the front of the
+ /// queue.
+ //-----------------------------------------------------------------------
+ T& pop() {
+ Mutex::Locker lock(m_mutex);
+ while(m_queue.empty()) {
+ m_condition.Wait(m_mutex);
+ }
+ T& item = m_queue.front();
+ m_queue.pop();
+ return item;
+ }
+
+ //-----------------------------------------------------------------------
+ /// Pushes an element to the back of the queue.
+ ///
+ /// Appends an element to the queue. If any threads are blocked inside a
+ /// pop() call (for an empty queue) this function unblocks one of them.
+ //-----------------------------------------------------------------------
+ void push(const T& item) {
+ Mutex::Locker lock(m_mutex);
+ m_queue.push(item);
+ m_condition.Signal();
+ }
+
+ //-----------------------------------------------------------------------
+ /// Queries if the queue is empty.
+ ///
+ /// @return
+ /// True if the queue has no elements, false otherwise.
+ //-----------------------------------------------------------------------
+ bool empty() {
+ Mutex::Locker lock(m_mutex);
+ return m_queue.empty();
+ }
+
+private:
+ std::queue<T> m_queue;
+ lldb_private::Mutex m_mutex;
+ lldb_private::Condition m_condition;
+};
+
+} // namespace lldb_private
+
+#endif // defined LLDB_PRIVATE_HOST_QUEUE
Index: source/Plugins/Process/Linux/ProcessMonitor.cpp
===================================================================
--- source/Plugins/Process/Linux/ProcessMonitor.cpp
+++ source/Plugins/Process/Linux/ProcessMonitor.cpp
@@ -37,7 +37,6 @@
#include "ProcessPOSIXLog.h"
#include "ProcessMonitor.h"
-
#define DEBUG_PTRACE_MAXBYTES 20
// Support ptrace extensions even when compiled without required kernel support
@@ -936,22 +935,14 @@
m_operation_thread(LLDB_INVALID_HOST_THREAD),
m_monitor_thread(LLDB_INVALID_HOST_THREAD),
m_pid(LLDB_INVALID_PROCESS_ID),
- m_terminal_fd(-1),
- m_client_fd(-1),
- m_server_fd(-1)
+ m_terminal_fd(-1)
{
+ sem_init(&m_operation_semaphore, 0, 0);
std::unique_ptr<LaunchArgs> args;
args.reset(new LaunchArgs(this, module, argv, envp,
stdin_path, stdout_path, stderr_path, working_dir));
- // Server/client descriptors.
- if (!EnableIPC())
- {
- error.SetErrorToGenericError();
- error.SetErrorString("Monitor failed to initialize.");
- }
-
StartLaunchOpThread(args.get(), error);
if (!error.Success())
return;
@@ -995,22 +986,13 @@
m_operation_thread(LLDB_INVALID_HOST_THREAD),
m_monitor_thread(LLDB_INVALID_HOST_THREAD),
m_pid(LLDB_INVALID_PROCESS_ID),
- m_terminal_fd(-1),
-
- m_client_fd(-1),
- m_server_fd(-1)
+ m_terminal_fd(-1)
{
+ sem_init(&m_operation_semaphore, 0, 0);
std::unique_ptr<AttachArgs> args;
args.reset(new AttachArgs(this, pid));
- // Server/client descriptors.
- if (!EnableIPC())
- {
- error.SetErrorToGenericError();
- error.SetErrorString("Monitor failed to initialize.");
- }
-
StartAttachOpThread(args.get(), error);
if (!error.Success())
return;
@@ -1246,19 +1228,6 @@
return args->m_error.Success();
}
-bool
-ProcessMonitor::EnableIPC()
-{
- int fd[2];
-
- if (socketpair(AF_UNIX, SOCK_STREAM, 0, fd))
- return false;
-
- m_client_fd = fd[0];
- m_server_fd = fd[1];
- return true;
-}
-
void
ProcessMonitor::StartAttachOpThread(AttachArgs *args, lldb_private::Error &error)
{
@@ -1926,78 +1895,26 @@
ProcessMonitor::ServeOperation(OperationArgs *args)
{
int status;
- pollfd fdset;
ProcessMonitor *monitor = args->m_monitor;
- fdset.fd = monitor->m_server_fd;
- fdset.events = POLLIN | POLLPRI;
- fdset.revents = 0;
-
// We are finised with the arguments and are ready to go. Sync with the
// parent thread and start serving operations on the inferior.
sem_post(&args->m_semaphore);
- for (;;)
- {
- if ((status = poll(&fdset, 1, -1)) < 0)
- {
- switch (errno)
- {
- default:
- assert(false && "Unexpected poll() failure!");
- continue;
-
- case EINTR: continue; // Just poll again.
- case EBADF: return; // Connection terminated.
- }
- }
-
- assert(status == 1 && "Too many descriptors!");
-
- if (fdset.revents & POLLIN)
- {
- Operation *op = NULL;
-
- READ_AGAIN:
- if ((status = read(fdset.fd, &op, sizeof(op))) < 0)
- {
- // There is only one acceptable failure.
- assert(errno == EINTR);
- goto READ_AGAIN;
- }
- if (status == 0)
- continue; // Poll again. The connection probably terminated.
- assert(status == sizeof(op));
- op->Execute(monitor);
- write(fdset.fd, &op, sizeof(op));
- }
+ for(;;) {
+ Operation *op = monitor->m_operation_queue.pop();
+ op->Execute(monitor);
+ sem_post(&monitor->m_operation_semaphore);
}
}
void
ProcessMonitor::DoOperation(Operation *op)
{
- int status;
- Operation *ack = NULL;
Mutex::Locker lock(m_server_mutex);
-
- // FIXME: Do proper error checking here.
- write(m_client_fd, &op, sizeof(op));
-
-READ_AGAIN:
- if ((status = read(m_client_fd, &ack, sizeof(ack))) < 0)
- {
- // If interrupted by a signal handler try again. Otherwise the monitor
- // thread probably died and we have a stale file descriptor -- abort the
- // operation.
- if (errno == EINTR)
- goto READ_AGAIN;
- return;
- }
-
- assert(status == sizeof(ack));
- assert(ack == op && "Invalid monitor thread response!");
+ m_operation_queue.push(op);
+ sem_wait(&m_operation_semaphore);
}
size_t
@@ -2188,8 +2105,7 @@
StopMonitoringChildProcess();
StopOpThread();
CloseFD(m_terminal_fd);
- CloseFD(m_client_fd);
- CloseFD(m_server_fd);
+ sem_destroy(&m_operation_semaphore);
}
void
Index: source/Plugins/Process/Linux/ProcessMonitor.h
===================================================================
--- source/Plugins/Process/Linux/ProcessMonitor.h
+++ source/Plugins/Process/Linux/ProcessMonitor.h
@@ -18,6 +18,7 @@
// Other libraries and framework includes
#include "lldb/lldb-types.h"
#include "lldb/Host/Mutex.h"
+#include "lldb/Host/Queue.h"
namespace lldb_private
{
@@ -188,15 +189,16 @@
private:
ProcessLinux *m_process;
+ lldb_private::Queue<Operation*> m_operation_queue;
+ sem_t m_operation_semaphore; // Posted to once operation complete.
+
lldb::thread_t m_operation_thread;
lldb::thread_t m_monitor_thread;
lldb::pid_t m_pid;
int m_terminal_fd;
lldb_private::Mutex m_server_mutex;
- int m_client_fd;
- int m_server_fd;
struct OperationArgs
{
@@ -244,9 +246,6 @@
static bool
Launch(LaunchArgs *args);
- bool
- EnableIPC();
-
struct AttachArgs : OperationArgs
{
AttachArgs(ProcessMonitor *monitor,
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
