Re: [PATCH 3/3] Fix control-c handling on Windows

[Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>]

>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> I've now tested this a bit, it's a big improvement.

Thanks for trying it.  I appreciate that.

Hannes> When I first started a program with 'new-console on', then the
Hannes> sigint_ours variable would not be initialized, and I would later get
Hannes> a crash on C-c.

I looked at this and my feeling is that this is a latent bug.

I think what's going on is that sigint_ours is set under different
conditions than it is used.

That is, it is set like:

  if (gdb_has_a_terminal ()
      && tinfo->ttystate != NULL
      && sharing_input_terminal (inf))
[...]
      if (!job_control)
	{
	  sigint_ours = install_sigint_handler (SIG_IGN);
[...]
      gdb_tty_state = target_terminal_state::is_inferior;


However later it is used:

  if (gdb_tty_state != desired_state)
[...]
      if (!job_control && desired_state == target_terminal_state::is_ours)
	{
	  install_sigint_handler (sigint_ours);

It's maybe hard to reason about but it seems to me that there's some
possibility for the value to be used even though it hasn't been set, and
I suspect that is what you are seeing.

It might be useful if you could confirm this.  Just some simple logging
at these two points would be sufficient.

If that's the issue then I can write a patch to change sigint_ours to be
a gdb::optional and check it that way.

Hannes> And for some reason one of my builds (x86_64+TUI+python) needed the
Hannes> #include <signal.h> in mingw-hdep.c, but my other (i686 basic) didn't.

I didn't see this but I went ahead and added the include to my patch,
since it seems harmless.

Hannes> But my basic i686 build had another problem when starting a program with
Hannes> 'new-console on', because at program start it called install_sigint_handler(),
Hannes> but rl_set_signals() would later override the SIGINT handler again, so C-c
Hannes> didn't work work in this situation.
Hannes> With 'new-console off' this didn't happen, since install_sigint_handler() was
Hannes> again called later since it shared the console.

I really don't understand the interaction between signal and
SetConsoleCtrlHandler.  I tried searching for some docs on this but
didn't find anything that was really enlightening.

Also it's somewhat surprising that the x86 and x86-64 builds would be
different in this regard.

Anyway ... I'm not sure what to do here yet.  The interactions with
readline are pretty hard to understand.  I guess the question is where
should install_sigint_handler be called where it is not called -- that
is, to reset the signals from readline.  Alternatively, where is it
called on x86-64 but not on x86?

I did try 'new-console on' with my (x86-64) build, and that worked fine.
I can try an x86 build and see if that works any better.

Tom