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

[Hannes Domani <ssbssa@yahoo.de>]

 Am Montag, 5. Dezember 2022, 19:57:36 MEZ hat Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:

> As Hannes pointed out, the Windows target-async patches broke C-c
> handling there.  Looking into this, I found a few oddities, fixed
> here.
>
> First, windows_nat_target::interrupt calls GenerateConsoleCtrlEvent.
> I think this event can be ignored by the inferior, so it's not a great
> way to interrupt.  Instead, using DebugBreakProcess (or a more
> complicated thing for Wow64) seems better.
>
> Second, windows_nat_target did not implement the pass_ctrlc method.
> Implementing this lets us remove the special code to call
> SetConsoleCtrlHandler and instead integrate into gdb's approach to C-c
> handling.  I believe that this should also fix the race that's
> described in the comment that's being removed.
>
> Initially, I thought a simpler version of this patch would work.
> However, I think what happens is that some other library (I'm not sure
> what) calls SetConsoleCtrlHandler while gdb is running, and this
> intercepts and handles C-c -- so that the gdb SIGINT handler is not
> called.  C-break continues to work, presumably because whatever
> handler is installed ignores it.
>
> This patch works around this issue by ensuring that the gdb handler
> always comes first.

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

Now it even works to interrupt a GUI program that was started with
'new-console off', that didn't work before.

But I did notice a few problems:

1)

When I first started a program with 'new-console on', then the
sigint_ours variable would not be initialized, and I would later get
a crash on C-c.
I've fixed it like this:

diff --git a/gdb/inflow.c b/gdb/inflow.c
index f9926122099..50c93b6e15a 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -165,7 +165,7 @@ static struct terminal_info *get_inflow_inferior_data (struct inferior *);
    save our handlers in these two variables and set SIGINT and SIGQUIT
    to SIG_IGN.  */
 
-static sighandler_t sigint_ours;
+static sighandler_t sigint_ours = SIG_IGN;
 #ifdef SIGQUIT
 static sighandler_t sigquit_ours;
 #endif
@@ -805,7 +805,8 @@ child_terminal_ours_1 (target_terminal_state desired_state)
         }
     }
 
-      if (!job_control && desired_state == target_terminal_state::is_ours)
+      if (!job_control && desired_state == target_terminal_state::is_ours
+      && sigint_ours != SIG_IGN)
     {
       install_sigint_handler (sigint_ours);
 #ifdef SIGQUIT

Not sure if there is a better solution.


2)

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


3)

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


That's all I've seen so far, thanks for this.


Hannes