Re: [patch] Fix SIGTERM signal safety (PR gdb/15358)

Jan Kratochvil
2013-07-25T15:41:07Z

On Thu, 25 Jul 2013 17:33:30 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>
> Jan> void
> Jan> handle_sigterm (int sig)
> Jan> {
> Jan> signal (sig, handle_sigterm);
> Jan> - quit_force ((char *) 0, stdin == instream);
> Jan> +
> Jan> + if (target_can_async_p ())
> Jan> + mark_async_signal_handler (async_sigterm_token);
> Jan> + else
> Jan> + {
> Jan> + sync_quit_force_run = 1;
> Jan> + set_quit_flag ();
>
> I think calling set_quit_flag here may do the wrong thing in one case.
>
> If Python is enabled, set_quit_flag sets a flag in the Python
> interpreter. This will cause a KeyboardInterrupt exception to be raised
> if the Python interpreter happens to be the code checking the flag
> first.
>
> This means that Python would raise this exception, probably not what is
> meant. And then, since sync_quit_force_run is set, you'd still get the
> quit_force call sometime after re-entering gdb anyhow.

This is SIGTERM. We do not care much what happens with GDB, we just need to
ensure GDB does not crash and that GDB removes breakpoints from the inferior.

I am roughly aware it may somehow abort Python scripts but IMO that is what we
want to do.

> It seems to me that simply not calling set_quit_flag ought to be safe
> here. QUIT checks the sync_quit_force_run already, so there doesn't
> seem to be a need to set both flags.

Without aborting Python the Python code may run for too long before QUIT; may
get checked again back in GDB.

> I think this would fix the Python problem.

Could you be more specific what is the Python problem? It is expected SIGTERM
is not a nice signal.

Thanks,
Jan