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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to