[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D132578#3751968 <https://reviews.llvm.org/D132578#3751968>, @mgorny wrote:

> Well, back when working on the async thread, you've indicated that it's a bad idea to read from one thread while writing from another, so I've basically focused on getting to the point when all reads and writes are guaranteed to happen from a single thread.

Ok, I think I know what you mean. That comment was made in the context of the GDB communication class, whose functionality is more complex (sending a packet involves both reading and writing) than the plain `Communication`.  Sending packets from multiple threads is more complex that a hypothetical simple setup where you have a single thread just doing reading, and another thread just for writing...

In D132578#3751969 <https://reviews.llvm.org/D132578#3751969>, @mgorny wrote:

> (with the final goal of using `StartReadThread()` inside gdb-remote comms to take care of reading the data for concurrent process plugin instances)

... but we don't have such a case, so doing this might be fine. I sort of understand the direction you're going with this, but I can't really visualize the final state. Could you give a short summary of the desired final architecture.

FWIW, my idea was to use the gdb-remote "async" thread for this centralized sort of reading(sending)/writing(receiving). The async thread serves a similar purpose to the communication read thread, but is completely unrelated to it. I thought that would be the easiest to implement, since we already kind of have a synchronization mechanism (the "continue" lock) between these thread and other threads. However, I don't have a complete picture of how would this work (I'd guess we have a single "async" thread for all of the processes, and it would somehow synchronize/arbitrate between them), so I am open to other ideas as well.


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

https://reviews.llvm.org/D132578