This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/4]: Handle SIGINT under Python by raising KeyboardInterrupt
>>>>> "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--;