Fix race condition in process resume

[labath]

Summary

Gdb-remote's async thread sent out the eBroadcastBitRunPacketSent message *before* actually
sending out the continue packet. Since it's this message the actually triggers the public state
transition, it could happen (and it did happen in TestAttachResume, which does an "process
interrupt" right after a continue) that we attempt to stop the inferior before it was actually
started (which obviously did not end well). This fixes the problem by moving the broadcast after
the packet was actually sent.

[clayborg]

Looks good as long as we always want to send the broadcast even if the continue packet fails to send.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

1074–1079 ↗

(On Diff #38429)

Do we want this to happen even if we fail to send the packet? Should this code be in the else clause above?

[labath]

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

1074–1079 ↗

(On Diff #38429)

It was always sent before this change, so I'd probably keep it that way. If the sending fails, we'll have to tear down the whole process anyway.

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

)
     m_curr_tid = LLDB_INVALID_THREAD_ID;
     Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS));
     if (log)
         log->Printf ("GDBRemoteCommunicationClient::%s ()", __FUNCTION__);
 
     Mutex::Locker locker(m_sequence_mutex);
     StateType state = eStateRunning;
 
-    BroadcastEvent(eBroadcastBitRunPacketSent, NULL);
     m_public_is_running.SetValue (true, eBroadcastNever);
     // Set the starting continue packet into "continue_packet". This packet
     // may change if we are interrupted and we continue after an async packet...
     std::string continue_packet(payload, packet_length);
 
     const auto sigstop_signo = process->GetUnixSignals()->GetSignalNumberFromName("SIGSTOP");
     const auto sigint_signo = process->GetUnixSignals()->GetSignalNumberFromName("SIGINT");
 
     bool got_async_packet = false;
+    bool broadcast_sent = false;
     
     while (state == eStateRunning)
     {
         if (!got_async_packet)
         {
             if (log)
                 log->Printf ("GDBRemoteCommunicationClient::%s () sending continue packet: %s", __FUNCTION__, continue_packet.c_str());
             if (SendPacketNoLock(continue_packet.c_str(), continue_packet.size()) != PacketResult::Success)
                 state = eStateInvalid;
             else
                 m_interrupt_sent = false;
         
+            if (! broadcast_sent)
+            {
+                BroadcastEvent(eBroadcastBitRunPacketSent, NULL);
+                broadcast_sent = true;
+            }
+
             m_private_is_running.SetValue (true, eBroadcastAlways);
         }
         
         got_async_packet = false;
 
         if (log)
             log->Printf ("GDBRemoteCommunicationClient::%s () ReadPacket(%s)", __FUNCTION__, continue_packet.c_str());
 

lldb/trunk/test/functionalities/attach_resume/TestAttachResume.py

 
 class AttachResumeTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @skipIfRemote
     @expectedFailureFreeBSD('llvm.org/pr19310')
     @expectedFailureWindows("llvm.org/pr24778")
-    @expectedFlakeyLinux('llvm.org/pr19310')
     def test_attach_continue_interrupt_detach(self):
         """Test attach/continue/interrupt/detach"""
         self.build()
         self.process_attach_continue_interrupt_detach()
 
     def process_attach_continue_interrupt_detach(self):
         """Test attach/continue/interrupt/detach"""