Author: jimingham Date: 2023-11-30T09:48:04-08:00 New Revision: d1bf1947e4e4f3ef75f2ba3ac9aa77dc38214de1
URL: https://github.com/llvm/llvm-project/commit/d1bf1947e4e4f3ef75f2ba3ac9aa77dc38214de1 DIFF: https://github.com/llvm/llvm-project/commit/d1bf1947e4e4f3ef75f2ba3ac9aa77dc38214de1.diff LOG: Send an explicit interrupt to cancel an attach waitfor. (#72565) Currently when you interrupt a: (lldb) process attach -w -n some_process lldb just closes the connection to the stub and kills the lldb_private::Process it made for the attach. The stub at the other end notices the connection go down and exits because of that. But when communication to a device is handled through some kind of proxy server which isn't as well behaved as one would wish, that signal might not be reliable, causing debugserver to persist on the machine, waiting to steal the next instance of that process. We can work around those failures by sending an explicit interrupt before closing down the connection. The stub will also have to be waiting for the interrupt for this to make any difference. I changed debugserver to do that. I didn't make the equivalent change in lldb-server. So long as you aren't faced with a flakey connection, this should not be necessary. Added: lldb/test/API/python_api/process/cancel_attach/TestCancelAttach.py Modified: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/source/Target/Process.cpp lldb/tools/debugserver/source/DNB.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index e653ef5d8ac54e4..9648f1fd52cf7cc 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2371,8 +2371,10 @@ Status ProcessGDBRemote::DoHalt(bool &caused_stop) { Status error; if (m_public_state.GetValue() == eStateAttaching) { - // We are being asked to halt during an attach. We need to just close our - // file handle and debugserver will go away, and we can be done... + // We are being asked to halt during an attach. We used to just close our + // file handle and debugserver will go away, but with remote proxies, it + // is better to send a positive signal, so let's send the interrupt first... + caused_stop = m_gdb_comm.Interrupt(GetInterruptTimeout()); m_gdb_comm.Disconnect(); } else caused_stop = m_gdb_comm.Interrupt(GetInterruptTimeout()); diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 21b80b8240ab64b..f3da2839e262e23 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -3156,8 +3156,8 @@ Status Process::Halt(bool clear_thread_plans, bool use_run_lock) { // Don't hijack and eat the eStateExited as the code that was doing the // attach will be waiting for this event... RestoreProcessEvents(); - SetExitStatus(SIGKILL, "Cancelled async attach."); Destroy(false); + SetExitStatus(SIGKILL, "Cancelled async attach."); return Status(); } @@ -3843,6 +3843,13 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) { ") woke up with an interrupt while attaching - " "forwarding interrupt.", __FUNCTION__, static_cast<void *>(this), GetID()); + // The server may be spinning waiting for a process to appear, in which + // case we should tell it to stop doing that. Normally, we don't NEED + // to do that because we will next close the communication to the stub + // and that will get it to shut down. But there are remote debugging + // cases where relying on that side-effect causes the shutdown to be + // flakey, so we should send a positive signal to interrupt the wait. + Status error = HaltPrivate(); BroadcastEvent(eBroadcastBitInterrupt, nullptr); } else if (StateIsRunningState(m_last_broadcast_state)) { LLDB_LOGF(log, diff --git a/lldb/test/API/python_api/process/cancel_attach/TestCancelAttach.py b/lldb/test/API/python_api/process/cancel_attach/TestCancelAttach.py new file mode 100644 index 000000000000000..9172c3d333fd1bd --- /dev/null +++ b/lldb/test/API/python_api/process/cancel_attach/TestCancelAttach.py @@ -0,0 +1,57 @@ +""" +Test using SendAsyncInterrupt to interrupt an "attach wait" +""" + +import lldb +import sys +import time +import threading +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +import lldbsuite.test.lldbutil + + +class AttachCancelTestCase(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def test_scripted_implementation(self): + """Test that cancelling a stuck "attach waitfor" works.""" + # First make an empty target for the attach: + target = self.dbg.CreateTarget(None) + + # We need to cancel this, so we need to do the attach + # on a separate thread: + class AttachThread(threading.Thread): + def __init__(self, target, error): + # Make this a daemon thread so if we don't manage to interrupt, + # Python will keep this thread from hanging the test. + threading.Thread.__init__(self, daemon=True) + self.target = target + self.error = error + + def run(self): + self.target.AttachToProcessWithName(lldb.SBListener(), "LLDB-No-Such-Process", True, self.error) + + error = lldb.SBError() + thread = AttachThread(target, error) + thread.start() + + # Now wait till the attach on the child thread has made a process + # for the attach attempt: + while not target.process.IsValid(): + time.sleep(1) + # I don't have a positive signal for "we started the attach attempt" + # so the best I can do is sleep a bit more here to give that a chance + # to start: + time.sleep(1) + + # Now send the attach interrupt: + target.process.SendAsyncInterrupt() + # We don't want to stall if we can't interrupt, so join with a timeout: + thread.join(60) + if thread.is_alive(): + self.fail("The attach thread is alive after timeout interval") + + # Now check the error, should say the attach was interrupted: + self.assertTrue(error.Fail(), "We succeeded in not attaching") + diff --git a/lldb/tools/debugserver/source/DNB.cpp b/lldb/tools/debugserver/source/DNB.cpp index f6c1130fdd8054c..0ec50df42d1fedc 100644 --- a/lldb/tools/debugserver/source/DNB.cpp +++ b/lldb/tools/debugserver/source/DNB.cpp @@ -50,6 +50,7 @@ #include "MacOSX/MachProcess.h" #include "MacOSX/MachTask.h" #include "MacOSX/ThreadInfo.h" +#include "RNBRemote.h" typedef std::shared_ptr<MachProcess> MachProcessSP; typedef std::map<nub_process_t, MachProcessSP> ProcessMap; @@ -745,7 +746,6 @@ DNBProcessAttachWait(RNBContext *ctx, const char *waitfor_process_name, break; } } else { - // Get the current process list, and check for matches that // aren't in our original list. If anyone wants to attach // to an existing process by name, they should do it with @@ -799,7 +799,33 @@ DNBProcessAttachWait(RNBContext *ctx, const char *waitfor_process_name, break; } - ::usleep(waitfor_interval); // Sleep for WAITFOR_INTERVAL, then poll again + // Now we're going to wait a while before polling again. But we also + // need to check whether we've gotten an event from the debugger + // telling us to interrupt the wait. So we'll use the wait for a possible + // next event to also be our short pause... + struct timespec short_timeout; + DNBTimer::OffsetTimeOfDay(&short_timeout, 0, waitfor_interval); + uint32_t event_mask = RNBContext::event_read_packet_available + | RNBContext::event_read_thread_exiting; + nub_event_t set_events = ctx->Events().WaitForSetEvents(event_mask, + &short_timeout); + if (set_events & RNBContext::event_read_packet_available) { + // If we get any packet from the debugger while waiting on the async, + // it has to be telling us to interrupt. So always exit here. + // Over here in DNB land we can see that there was a packet, but all + // the methods to actually handle it are protected. It's not worth + // rearranging all that just to get which packet we were sent... + DNBLogError("Interrupted by packet while waiting for '%s' to appear.\n", + waitfor_process_name); + break; + } + if (set_events & RNBContext::event_read_thread_exiting) { + // The packet thread is shutting down, get out of here... + DNBLogError("Interrupted by packet thread shutdown while waiting for " + "%s to appear.\n", waitfor_process_name); + break; + } + } } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
