[Lldb-commits] [PATCH] Improve stability of ProcessMonitor -- use a thread-safe queue instead of file descriptors with unchecked read/write calls to transfer Operation* between threads.

[Malea, Daniel]

Thanks for taking a look Greg. You're right; no ownership is assumed by the Queue class. The caller of ProcessMonitor::DoOperation() continues to own the Operation object -- in most cases, the Operations are stack allocated anyways...


Dan



-----Original Message-----
From: Greg Clayton [mailto:gclayton at apple.com]
Sent: Friday, September 13, 2013 7:50 PM
To: reviews+D1682+public+ef3a1a1390e918a7 at llvm-reviews.chandlerc.com
Cc: Malea, Daniel; lldb-commits
Subject: Re: [Lldb-commits] [PATCH] Improve stability of ProcessMonitor --
use a thread-safe queue instead of file descriptors with unchecked
read/write calls to transfer Operation* between threads.

I am assuming the ownership of the "Operation*" objects is not assumed to
be handled by the new Queue class since it is a queue of pointers that won't
get freed?

Other than that it looks ok to me, but I haven't done much with
ProcessMonitor so another linux expert would be good to consult.

Greg


On Sep 13, 2013, at 3:25 PM, Daniel Malea <daniel.malea at intel.com> wrote:

> 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
> <D1682.1.patch>_______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits