RFC: clear quit_flag in quit, not throw_exception

RFC: clear quit_flag in quit, not throw_exception

[Tom Tromey]

I noticed that throw_exception clears quit_flag and immediate_quit.

This seems like it could cause gdb to miss C-c sometimes.
In particular, if a SIGINT arrives and then, before QUIT is called, some
code in gdb throws an unrelated exception, then the SIGINT will
effectively be ignored.

It seems to me that it is just as safe, and more correct, to clear these
flags in the quit function.

Tom

	* exceptions.c (throw_exception): Don't clear quit_flag or
	immediate_quit.
	* utils.c (quit): Clear quit_flag and immediate_quit.
---
 gdb/exceptions.c |    3 ---
 gdb/utils.c      |    3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index 7db9df9..84c21a2 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -221,9 +221,6 @@ exceptions_state_mc_action_iter_1 (void)
 void
 throw_exception (struct gdb_exception exception)
 {
-  quit_flag = 0;
-  immediate_quit = 0;
-
   do_cleanups (all_cleanups ());
 
   /* Jump to the containing catch_errors() call, communicating REASON
diff --git a/gdb/utils.c b/gdb/utils.c
index c69c3e0..6ba8c62 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1055,6 +1055,9 @@ print_sys_errmsg (const char *string, int errcode)
 void
 quit (void)
 {
+  quit_flag = 0;
+  immediate_quit = 0;
+
 #ifdef __MSDOS__
   /* No steenking SIGINT will ever be coming our way when the
      program is resumed.  Don't lie.  */
-- 
1.7.7.6

Re: RFC: clear quit_flag in quit, not throw_exception

[Pedro Alves]

On 08/02/2012 05:13 PM, Tom Tromey wrote:
> I noticed that throw_exception clears quit_flag and immediate_quit.
> 
> This seems like it could cause gdb to miss C-c sometimes.
> In particular, if a SIGINT arrives and then, before QUIT is called, some
> code in gdb throws an unrelated exception, then the SIGINT will
> effectively be ignored.
> 
> It seems to me that it is just as safe, and more correct, to clear these
> flags in the quit function.

If the unrelated exception/error unwinds things all the way up to the
top prompt, managing to bypass all QUIT calls, then we'll end up with
a dangling quit until the next command it entered (resulting in a spurious,
delayed "Quit"), it seems to me.  If that analysis is correct, maybe we should
also clear or handle these somehow somewhere near the top loop?

> 
> Tom
> 
> 	* exceptions.c (throw_exception): Don't clear quit_flag or
> 	immediate_quit.
> 	* utils.c (quit): Clear quit_flag and immediate_quit.
> ---
>  gdb/exceptions.c |    3 ---
>  gdb/utils.c      |    3 +++
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/exceptions.c b/gdb/exceptions.c
> index 7db9df9..84c21a2 100644
> --- a/gdb/exceptions.c
> +++ b/gdb/exceptions.c
> @@ -221,9 +221,6 @@ exceptions_state_mc_action_iter_1 (void)
>  void
>  throw_exception (struct gdb_exception exception)
>  {
> -  quit_flag = 0;
> -  immediate_quit = 0;
> -
>    do_cleanups (all_cleanups ());
>  
>    /* Jump to the containing catch_errors() call, communicating REASON
> diff --git a/gdb/utils.c b/gdb/utils.c
> index c69c3e0..6ba8c62 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1055,6 +1055,9 @@ print_sys_errmsg (const char *string, int errcode)
>  void
>  quit (void)
>  {
> +  quit_flag = 0;
> +  immediate_quit = 0;
> +
>  #ifdef __MSDOS__
>    /* No steenking SIGINT will ever be coming our way when the
>       program is resumed.  Don't lie.  */
> 


-- 
Pedro Alves

Re: RFC: clear quit_flag in quit, not throw_exception

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> If the unrelated exception/error unwinds things all the way up to
Pedro> the top prompt, managing to bypass all QUIT calls, then we'll end
Pedro> up with a dangling quit until the next command it entered
Pedro> (resulting in a spurious, delayed "Quit"), it seems to me.  If
Pedro> that analysis is correct, maybe we should also clear or handle
Pedro> these somehow somewhere near the top loop?

Thanks for noticing this.

I looked into the problem in more detail, and I think it is a crazy
morass.

The quit_flag part is all solvable.  Maybe I will tackle it.

However, the immediate_quit part is really quite bad.  I started by
changing the current places that set and clear immediate_quit to use a
cleanup instead.  However, remote_start_remote remains a big problem --
it is just wrong.  I think a fix here would be to only set and clear
immediate_quit around I/O primitives.  But, finding all those and
applying the change is troublesome.

One idea I had was that maybe we could simply not have immediate_quit,
but instead rely on EINTR and sprinkle more QUIT calls around.  In my
initial experiment, this didn't seem to work, but I haven't gone back
yet to understand why.

Maybe someone else has a more workable idea.

Tom