[Lldb-commits] [PATCH] D131160: [WIP][lldb] Add "event" capability to the MainLoop class

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for taking care of the posix part.

I'm sort of warming up to the idea of reusing pending callback machinery for this. Besides avoiding the awkward naming problem, it also avoid another somewhat strange situation where our EventHandle offer additional actions on the event (which is different from all our other handle types, which only support deletion). All it would take is to add some locking around the PendingCallback manipulations, and write to the pipe (or trigger the event) whenever a new callback is registered, so that the Run function can invoke it.

It's a slightly different programming paradigm, and I suppose it also depends on which mechanism would be more natural for our InterruptRead use case (have you tried implementing it using this?), but I think the two mechanisms should be equivalent (as in, one could implement one using the other -- fairly easily).

What do you think about that?

In D131160#3726698 <https://reviews.llvm.org/D131160#3726698>, @mgorny wrote:

> In D131160#3726642 <https://reviews.llvm.org/D131160#3726642>, @mgorny wrote:
>
>> Ok, I've just realized that the pipe writing solution isn't going to work when the notification happens from the same thread (as the unittest does right now). @labath, do you happen to have an idea how to solve that reasonably cleanly?
>
> Ah, nevermind, for some reason I thought pipe writes are going to block until read happens. Perhaps we don't need to worry about that.

A pipe has a fixed (on linux, it's configurable) capacity, and it should be sufficiently large that we don't have to worry about it. If we wanted to be super safe, we could make the writes non-blocking. It should be safe to ignore `EAGAIN`s in this situation, as that means the pipe already contains some unprocessed bytes, and all of the pending notifications will be processed once the reading thread "catches up".



================
Comment at: lldb/source/Host/posix/MainLoopPosix.cpp:230
+  const int event_pipe_fd = m_event_pipe.GetReadFileDescriptor();
+  m_read_fds.insert({event_pipe_fd, [this, event_pipe_fd](MainLoopBase &loop) {
+                       char c;
----------------
I like this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131160/new/

https://reviews.llvm.org/D131160