[Lldb-commits] [PATCH] D97017: [lldb-server] Exit the DataAvailableCallback loop when `done` or `interrupt` are set

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

What are the situations where we set `done` or `interrupt`? The ones I could find are:

- the `k` packet. This packet is (for good reasons) specified very vaguely. The spec says it should have no reply, but lldb will actually try to listen for a reply anyway. I think that making lldb-server stop sending the `k` reply might be a good thing (for protocol conformance), but that still doesn't mean we should exit immediately after receiving it. The ideal behavior would be to wait(2) for the inferior, and only exit after it has been reaped properly.
- the interrupt (^C) packet in the platform personality. I'm not sure that we ever actually send this packet on platform connections, but even if we did, exiting does not seem like the proper response.

So overall, I'm not sure that this patch is really a good idea.



================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1064
+
+    if (done || interrupt) {
+      m_mainloop.RequestTermination();
----------------
omjavaid wrote:
> GDBRemoteCommunicationServer::GetPacketAndSendResponse sets done on eServerPacketType_invalid (empty packet) wouldnt termination be too strict for that case. 
> 
Possibly. Although, I don't think a well-behaved client should ever be sending an empty packet, so terminating the connection when it does send it, wouldn't be too bad imo.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D97017