[Lldb-commits] [PATCH] D114529: [lldb/gdb-remote] Remove initial pipe-draining workaround

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Nov 24 05:22:26 PST 2021


labath created this revision.
labath added reviewers: emaste, mgorny, jasonmolenda.
labath requested review of this revision.
Herald added a project: LLDB.

This code, added in rL197579 <https://reviews.llvm.org/rL197579> (Dec 2013) is supposed to work around what
was presumably a qemu bug, where it would send unsolicited stop-reply
packets after the initial connection.

At present, qemu does not exhibit such behavior. Also, the 10ms delay
introduced by this code is sufficient to mask bugs in other stubs, but
it is not sufficient to *reliably* mask those bugs. This resulted in
flakyness in one of our stubs, which was (incorrectly) sending a +
packet at the start of the connection, resulting in a small-but-annoying
number of dropped connections.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114529

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -86,13 +86,6 @@
   std::chrono::steady_clock::time_point start_of_handshake =
       std::chrono::steady_clock::now();
   if (SendAck()) {
-    // Wait for any responses that might have been queued up in the remote
-    // GDB server and flush them all
-    StringExtractorGDBRemote response;
-    PacketResult packet_result = PacketResult::Success;
-    while (packet_result == PacketResult::Success)
-      packet_result = ReadPacket(response, milliseconds(10), false);
-
     // The return value from QueryNoAckModeSupported() is true if the packet
     // was sent and _any_ response (including UNIMPLEMENTED) was received), or
     // false if no response was received. This quickly tells us if we have a
@@ -106,17 +99,15 @@
           std::chrono::duration<double>(end_of_handshake - start_of_handshake)
               .count();
       if (error_ptr) {
-        if (packet_result == PacketResult::ErrorDisconnected)
+        if (!IsConnected())
           error_ptr->SetErrorString("Connection shut down by remote side "
                                     "while waiting for reply to initial "
                                     "handshake packet");
-        else if (packet_result == PacketResult::ErrorReplyTimeout)
+        else
           error_ptr->SetErrorStringWithFormat(
               "failed to get reply to handshake packet within timeout of "
               "%.1f seconds",
               handshake_timeout);
-        else
-          error_ptr->SetErrorString("failed to get reply to handshake packet");
       }
     }
   } else {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114529.389470.patch
Type: text/x-patch
Size: 1858 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20211124/b13b3efe/attachment.bin>


More information about the lldb-commits mailing list