[lldb-server] Exit the DataAvailableCallback loop when `done` or `interrupt` are set

[JDevlieghere]

Summary

When looking at GDBRemoteCommunicationServerLLGS::DataAvailableCallback I noticed that the done and interrupt flags are currently being ignored. I assumed the reason is that GetPacketAndSendResponse always returns either PacketResult::Success or PacketResult::ErrorReplyTimeout but there are plenty of code paths that return PacketResult::Success but set done or interrupt to true.

[omjavaid]

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

950

May be we can set m_exit_now instead of setting done, or interrupt. if we have to exit on empty response.

1064

GDBRemoteCommunicationServer::GetPacketAndSendResponse sets done on eServerPacketType_invalid (empty packet) wouldnt termination be too strict for that case.

[labath]

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.

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

1064

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.

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

               __FUNCTION__, process->GetID());
   }
 
   // Close the pipe to the inferior terminal i/o if we launched it and set one
   // up.
   MaybeCloseInferiorTerminalConnection();
 
   // We are ready to exit the debug monitor.
   m_exit_now = true;
   m_mainloop.RequestTermination();
 }
 
 void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped(
     NativeProcessProtocol *process) {
   assert(process && "process cannot be NULL");
 
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));

     if ((result != PacketResult::Success)) {
       LLDB_LOGF(log,
                 "GDBRemoteCommunicationServerLLGS::%s processing a packet "
                 "failed: %s",
                 __FUNCTION__, error.AsCString());
       m_mainloop.RequestTermination();
       break;
     }
+
+    if (done || interrupt) {
+      m_mainloop.RequestTermination();
+      break;
+    }
   }
 }
 
 Status GDBRemoteCommunicationServerLLGS::InitializeConnection(
     std::unique_ptr<Connection> connection) {
   IOObjectSP read_object_sp = connection->GetReadObject();
   GDBRemoteCommunicationServer::SetConnection(std::move(connection));