Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race [Re: Regression for attach from stdin [Re: [pushed] Re: [PATCH v6 0/2] enable target-async by default]]

[Pedro Alves <palves@redhat.com>]

On 07/29/2014 11:03 PM, Doug Evans wrote:
> A related issue (or the same one if one prefers):
> 
> post_create_inferior does this:
> 
>   /* Be sure we own the terminal in case write operations are performed.  */
>   target_terminal_ours ();

Yeah.

As I mentioned before, when I wrote a test that sends a ctrl-c right
after "attach", which triggered the issue mentioned in the new
comment,

     - installs a SIGINT handler that forwards SIGINT to the inferior.
       Otherwise a Ctrl-C pressed just while waiting for the initial
       stop would end up as a spurious Quit.

it tripped on a set of other races.

This call was one of them.  We should actually be calling
target_terminal_ours_for_output, not target_terminal_ours, so
that stdin/ctrl-c is still redirected to the inferior.  As is,
IIRC, if a ctrl-c arrives after than target_terminal_ours and before
the next target_terminal_inferior, we end up in the prompt, with
a Quit, but with the target still running.

I gave up fixing those at the time, as they're orthogonal to
sync/async.

> 
> but post_create_inferior is called *after* target_post_attach
> in attach_command_post_wait:
> 
>   /* Take any necessary post-attaching actions for this platform.  */
>   target_post_attach (ptid_get_pid (inferior_ptid));
> 
>   post_create_inferior (&current_target, from_tty);
> 
> What if target_post_attach does some writes?

Something should call target_terminal_ours_for_output before.

> Seems like it can, e.g., some of the ptrace checking stuff may print a warning.
> Plus attach_command_post_wait calls some other stuff before
> post_create_inferior that could cause some writes to the terminal.
> 
> Question: Is there a specific terminal state that needs to be in
> effect when attach_command_post_wait returns?
> Or can we just call target_terminal_ours at its start?

I think we can.

> [and leave it to other code to switch back to target_terminal_inferior
> as needed - e.g. proceed calls resume which will call
> target_terminal_inferior]

Even with that, it'll still be possible to have a ctrl-c pressed after
to_attach is called, and before target_terminal_inferior is called
(and therefore set_sigint_trap is called, which sets up ctrl-c
forwarding).

When I last looked at this, it seemed to be that ideally, we should
only have a single SIGINT handler, which just records that a ctrl-c was
pressed, and then the event loop decides whether a ctrl-c is a Quit or
whether it should be forwarded to the inferior, depending on whether the
target is running on the foreground, or not.

-- 
Thanks,
Pedro Alves