[Lldb-commits] [PATCH] D125575: [lldb] [llgs] Implement non-stop style stop notification packets

Wed Jun 15 12:55:59 PDT 2022

mgorny added inline comments.

================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3749
+    return SendErrorResponse(Status("No pending notification to ack"));
+  m_stop_notification_queue.pop_front();
+  if (!m_stop_notification_queue.empty())
----------------
labath wrote:
> Is this correct? I would expect to this to first read (`front()`) the packet before popping it.
Yes, it is. I suppose it could be a bit misleading. The first message is sent as asynchronous notification but we keep it on the queue (and technically, the protocol assumes we may resend the asynchronous notification if we think client may have not gotten it). `vStopped` means client has processed it and is ready for the next one, so we pop the one that's been sent already and send the next one, wait for the next ACK and so on.

================
Comment at: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py:1443
+            m = self.expect_gdbremote_sequence()
+            del m["O_content"]
+            threads = [m]
----------------
labath wrote:
> Is this necessary? (I get that it's not useful, but it seems weird...)
It's convenient because it lets us compare the matches from `threads` and `threads_verify`.

================
Comment at: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py:1525
+        procs = self.prep_debug_monitor_and_inferior(
+                inferior_args=["thread:new"])
+        self.test_sequence.add_log_lines(
----------------
labath wrote:
> Are you sure this is not racy (like, can the inferior terminate before we get a chance to interrupt it)?
Well, if I'm reading the code right, the new thread waits 60 seconds, so yes, technically it's racy ;-). Do you have a better idea? Some new argument that puts the process in an infinite sleep loop?

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

https://reviews.llvm.org/D125575