[PATCH 2/4]: Handle SIGINT under Python by raising KeyboardInterrupt

Tom Tromey tromey@redhat.com
Tue Jul 31 13:59:00 GMT 2012


>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:

Yit> But immediate_quit is set by command_line_input, which causes
Yit> async_request_quit to call quit (), which trips the TRY_CATCH in my
Yit> readline wrapper and sets PyExc_KeyboardInterrupt, leading to a
Yit> second exception.

Aha, thanks.  That was very clear.

Yit> It's seems strange to me that async_request_quit calls quit when
Yit> either check_quit_flag or immediate_quit is true, since it leads to
Yit> the possibility of quit being called twice, once if check_quit_flag is
Yit> tested, e.g., via QUIT, and a second time by the start_event_loop or
Yit> other calls to gdb_do_one_event, via "gdb_call_async_signal_handler
Yit> (sigint_token, immediate_quit)".

Yeah, I agree.  That does seem strange.

I am wondering if the appended patch is correct.
It removes the check of immediate_quit from async_request_quit.
It also changes all the places that set immediate_quit to immediately
call QUIT afterward; the idea being that this handles the race
situation.  It also changes prompt_for_continue to use quit rather than
a call to async_request_quit, which has the same effect but doesn't rely
on the handling of immediate_quit.

A couple notes though...

First, it seems to me that immediate_quit and quit_flag ought to be
of type sig_atomic_t.  And, immediate_quit should be set and cleared,
not incremented and decremented.  I assume these aren't really needed in
practice, they are just pedantically more correct.

Also, I wonder how well these changes would work on Windows.
>From reading the code it appears to be ok, but it has been a long time
since I worked on Windows.

Comments appreciated.

Tom

2012-07-31  Tom Tromey  <tromey@redhat.com>

	* utils.c (prompt_for_continue): Call QUIT.  Use quit, not
	async_request_quit.
	* top.c (command_line_input): Call QUIT.
	* remote.c (remote_start_remote): Call QUIT.
	* remote-mips.c (mips_expect_timeout): Call QUIT.
	* monitor.c (monitor_expect): Call QUIT.
	* event-top.c (async_request_quit): Don't check immediate_quit.

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 52e7852..779798f 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -828,10 +828,9 @@ async_request_quit (gdb_client_data arg)
   /* If the quit_flag has gotten reset back to 0 by the time we get
      back here, that means that an exception was thrown to unwind the
      current command before we got back to the event loop.  So there
-     is no reason to call quit again here, unless immediate_quit is
-     set.  */
+     is no reason to call quit again here.  */
 
-  if (quit_flag || immediate_quit)
+  if (quit_flag)
     quit ();
 }
 
diff --git a/gdb/monitor.c b/gdb/monitor.c
index b9f345e..f8cb8c62 100644
--- a/gdb/monitor.c
+++ b/gdb/monitor.c
@@ -512,6 +512,7 @@ monitor_expect (char *string, char *buf, int buflen)
     }
 
   immediate_quit++;
+  QUIT;
   while (1)
     {
       if (buf)
diff --git a/gdb/remote-mips.c b/gdb/remote-mips.c
index db4381b..7819f3f 100644
--- a/gdb/remote-mips.c
+++ b/gdb/remote-mips.c
@@ -588,6 +588,7 @@ mips_expect_timeout (const char *string, int timeout)
     }
 
   immediate_quit++;
+  QUIT;
   while (1)
     {
       int c;
diff --git a/gdb/remote.c b/gdb/remote.c
index 6780212..6549508 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3271,6 +3271,7 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
   char *wait_status = NULL;
 
   immediate_quit++;		/* Allow user to interrupt it.  */
+  QUIT;
 
   if (interrupt_on_connect)
     send_interrupt_sequence ();
diff --git a/gdb/top.c b/gdb/top.c
index 061ad48..386e5b1 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -942,6 +942,7 @@ command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
   /* Control-C quits instantly if typed while in this loop
      since it should not wait until the user types a newline.  */
   immediate_quit++;
+  QUIT;
 #ifdef STOP_SIGNAL
   if (job_control)
     signal (STOP_SIGNAL, handle_stop_sig);
diff --git a/gdb/utils.c b/gdb/utils.c
index c69c3e0..5987eaf 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1812,6 +1812,7 @@ prompt_for_continue (void)
   reinitialize_more_filter ();
 
   immediate_quit++;
+  QUIT;
   /* On a real operating system, the user can quit with SIGINT.
      But not on GO32.
 
@@ -1834,7 +1835,7 @@ prompt_for_continue (void)
       while (*p == ' ' || *p == '\t')
 	++p;
       if (p[0] == 'q')
-	async_request_quit (0);
+	quit ();
       xfree (ignore);
     }
   immediate_quit--;



More information about the Gdb-patches mailing list