[Lldb-commits] [PATCH] Prevent debugserver from sending two stop notifications on 'k' gdb-remote message

[Greg Clayton]

We end up with a race condition in LLDB if this happens. There are a few things that must happen though:
1 - We need to get a "X09" response back from debugserver in a timely manor
2 - debugserver must live long enough to reap the child process

The call to DNBProcessKill() is synchronous, so by the time it returns the process should be exited. So we should ensure that only one copy of the "X09" packet gets sent by having RNBRemote verify if this has already been sent or not, and sending it only if it hasn't. 

We ran into problems before if we didn't send this and I don't know what those problems were and if they will re-occur, so I would prefer not to make this debugserver change and just have you adjust your test case for debugserver to avoid any potential problems.

Greg

> On Jul 14, 2014, at 1:04 PM, Todd Fiala <todd.fiala at gmail.com> wrote:
> 
> Currently debugserver sends two responses to the 'k' gdb-remote kill request.  The first one is automatically sent out without question when receiving the 'k' packet.  The second one comes when the process really dies (i.e. the real result, including extra info re: description).
> 
> This change removes the first automatic 'X09' response, and adjusts the gdb-remote 'k' test to accept the key-val info from the true X response from death of the process on MacOSX.  This change also eliminates a set of verbose warnings from the gdb-remote test suite where the second X results was picked up but was unexpected, never matched, by the test system.
> 
> llgs already behaves this way.
> 
> Index: test/tools/lldb-gdbserver/TestLldbGdbServer.py
> ===================================================================
> --- test/tools/lldb-gdbserver/TestLldbGdbServer.py	(revision 212951)
> +++ test/tools/lldb-gdbserver/TestLldbGdbServer.py	(working copy)
> @@ -408,10 +408,10 @@
>  
>      def attach_commandline_kill_after_initial_stop(self):
>          procs = self.prep_debug_monitor_and_inferior()
> -        self.test_sequence.add_log_lines(
> -            ["read packet: $k#6b",
> -             "send packet: $X09#00"],
> -            True)
> +        self.test_sequence.add_log_lines([
> +            "read packet: $k#6b",
> +            {"direction":"send", "regex":r"^\$X[0-9a-fA-F]+([^#]*)#[0-9A-Fa-f]{2}" },
> +            ], True)
>          self.expect_gdbremote_sequence()
>  
>          # Wait a moment for completed and now-detached inferior process to clear.
> Index: tools/debugserver/source/RNBRemote.cpp
> ===================================================================
> --- tools/debugserver/source/RNBRemote.cpp	(revision 212951)
> +++ tools/debugserver/source/RNBRemote.cpp	(working copy)
> @@ -3857,7 +3857,8 @@
>      // No response to should be sent to the kill packet
>      if (m_ctx.HasValidProcessID())
>          DNBProcessKill (m_ctx.ProcessID());
> -    SendPacket ("X09");
> +    // Don't send the X - wait for the real process death to do that.
> +    // Otherwise, we get two X stop notifications for the process death.
>      return rnb_success;
>  }
>  
> -- 
> -Todd
> <patch_gdbremote_k_one_response.diff>_______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits