[Lldb-commits] [lldb] [lldb] Ignore async notification packets while waiting for a response (PR #202556)

[Jason Molenda via lldb-commits]

jasonmolenda wrote:

Hi, thanks for working on this issue.

I'm a little uncertain of the structure of the change in `GDBRemoteCommunication::WaitForPacketNoLock`.  The code as it exists today is

```
  uint8_t buffer[8192];
  while (IsConnected() && !timed_out) {
    size_t bytes_read = Read(buffer, sizeof(buffer), timeout, status, &error);
    if (bytes_read > 0) {
      if (CheckForPacket(buffer, bytes_read, packet) != PacketType::Invalid)
        return PacketResult::Success;
    } else {
       // PacketType::Invalid
            case eConnectionStatusTimedOut:
          timed_out = true;
```

So we read up-to 8192 bytes, or we stop when a timeout has been reached.  If we got any data, we pass it in to `CheckForPacket` which appends it to the incoming-byte-buffer.  We may be receiving more than 8192 bytes worth of packet, so we continue looping here, getting the next 8192 bytes and adding it to the internal incoming-packet buffer until we get a complete packet.

This PR changes this loop to read

```
    size_t bytes_read = Read(buffer, sizeof(buffer), timeout, status, &error);
    if (bytes_read > 0) {

   packet_type = CheckForPacket(buffer, bytes_read, packet);
    while (packet_type == PacketType::Notify)
         packet_type = CheckForPacket(nullptr, 0, packet);
```

The `Read()` receives an async-notify packet.  `CheckForPacket` appends it to the internal buffer, sees that we have a complete packet, returns `PacketType::Notify`.  Now we check if there is *another* async-notify packet, or real response, that we received at the same time in that original `Read`.  

But what if it hasn't been sent yet?  I'm guessing that a stub doing this might send an async-notify packet every 500ms or something, so we'll only receive the first one in that first `Read`, the `while` loop exits because `CheckForPacket()` returns
```
  packet.Clear();
  return GDBRemoteCommunication::PacketType::Invalid;
```
when the incoming packet buffer (`m_bytes.empty()`) is empty.  This exits the `while (packet_type == PacketType::Notify)` loop and we return a failure.

I think the only way this works is in the style of the unit test,

```
  ASSERT_TRUE(Write("%oocd_keepalive:01#55%oocd_keepalive:02#56$OK#9a"));
```

where one or more async-notifies AND the final actual packet result are all received together.

https://github.com/llvm/llvm-project/pull/202556