[Lldb-commits] [PATCH] D132283: [lldb] [Core] Reimplement Communication::ReadThread using MainLoop

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Core/Communication.cpp:427
   // Notify the read thread.
-  m_connection_sp->InterruptRead();
 
----------------
mgorny wrote:
> labath wrote:
> > Have you considered putting this code (some version of it) inside `InterruptRead`? Basically replacing the `select` call inside BytesAvailable with something MainLoop-based ?
> To be honest, I've been considering removing `InterruptRead()` entirely, now that the read loop is triggered by available input rather than read-with-timeout. However, it's still used by editline support.
> 
> That said, I'm not sure what's your idea, given that `Connection` does not have awareness of `Communication` that's using it. I suppose I could pass the `MainLoop` instance as a parameter but we'd still have to maintain a separate version for editline support, and we only have a single caller here.
> To be honest, I've been considering removing `InterruptRead()` entirely, now that the read loop is triggered by available input rather than read-with-timeout. However, it's still used by editline support.
If we could do that, then it would be even better. And it looks like it should be possible to use a MainLoop instance inside the Editline class, instead of the built-in interruption support.

> That said, I'm not sure what's your idea, given that `Connection` does not have awareness of `Communication` that's using it. I suppose I could pass the `MainLoop` instance as a parameter but we'd still have to maintain a separate version for editline support, and we only have a single caller here.

I don't claim to have thought this out completely, but I was imagining that the MainLoop instance would be internal to the Connection class. The external interface of the Connection class would remain unchanged (so the Communication class would not need to change), and the InterruptRead function would be implemented using the MainLoop functionality.


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

https://reviews.llvm.org/D132283