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

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Well, my "baseline" idea was something like that: having a shared (e.g. via `shared_ptr`) `m_gdb_comm`, and running async thread as part of that. The thread would read packets from remote and split them into two groups: asynchronous events (i.e. stop reasons and related packets) and synchronous replies. This is reasonably easy for non-stop protocol since the former are always sent as `%` notification, while the latter as regular packets. For non-nonstop case, I suppose we would just trigger some explicit switch on resuming/leaving resume. Asynchronous events would be passed (e.g. via callback) to process instances that are currently resumed. Synchronous replies would be just passed to whichever process is currently waiting for the reply.

However, the problem with that design is that it requires a lot of different changes, and the gdb-remote plugin code is not exactly friendly to that. So I was trying to split it into smaller steps that would be testable and reviewable on their own, i.e. basically refactor, refactor, get multiprocess working, cleanup, cleanup. What these changes were aiming at was basically moving all the reads and writes from gdb-remote into a dedicated thread reusing the code already in LLDB, so I could initially add support for the new "async" thread and handling the resume-related packets there without having to immediately replace the existing code for synchronous packets that reads and writes to `m_gdb_comm` directly from a lot of places. However, now that I think of it, I didn't really think it through and using read thread along with some trivial locking and queues won't be sufficient to be able to cleanly supports reading these two kinds of packets from different threads.

At this point I'm basically lost in the woods and I'm not really sure how to proceed, or how much of this code is actually worth further consideration.


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

https://reviews.llvm.org/D132578