public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* Handle SIGINT in Python
@ 2012-01-10 21:31 Khoo Yit Phang
  2012-01-10 21:47 ` Doug Evans
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Khoo Yit Phang @ 2012-01-10 21:31 UTC (permalink / raw)
  To: gdb-patches; +Cc: Khoo Yit Phang

Hi,

I've written a patch to GDB's Python support to allow SIGINT to interrupt a running script to address the bug http://sourceware.org/bugzilla/show_bug.cgi?id=13265 (I've attached my patch to that page).

It works by installing a Python-specific SIGINT handler just before executing a Python script, and restoring the original SIGINT handler after execution as well as at calls to gdb.execute(...).

There's one caveat: if an inferior is running via, e.g., gdb.execute("run"), SIGINT will interrupt the inferior but not the enclosing "python" command. I think it would be better to also interrupt the "python" command, but I'm not sure how to detect this case yet.

Yit
January 10, 2012

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-10 21:31 Handle SIGINT in Python Khoo Yit Phang
@ 2012-01-10 21:47 ` Doug Evans
  2012-01-10 22:09   ` Khoo Yit Phang
  2012-01-11 20:59   ` Tom Tromey
  2012-01-10 21:49 ` Tom Tromey
  2012-01-11 21:15 ` Tom Tromey
  2 siblings, 2 replies; 18+ messages in thread
From: Doug Evans @ 2012-01-10 21:47 UTC (permalink / raw)
  To: Khoo Yit Phang; +Cc: gdb-patches

On Tue, Jan 10, 2012 at 1:29 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> Hi,
>
> I've written a patch to GDB's Python support to allow SIGINT to interrupt a running script to address the bug http://sourceware.org/bugzilla/show_bug.cgi?id=13265 (I've attached my patch to that page).
>
> It works by installing a Python-specific SIGINT handler just before executing a Python script, and restoring the original SIGINT handler after execution as well as at calls to gdb.execute(...).
>
> There's one caveat: if an inferior is running via, e.g., gdb.execute("run"), SIGINT will interrupt the inferior but not the enclosing "python" command. I think it would be better to also interrupt the "python" command, but I'm not sure how to detect this case yet.

There is value in having the SIGINT *only* affect the inferior.
It's up to the script to handle the various reasons why the inferior
may have stopped, and you don't (generally) want to interfere with
that (by interrupting the script too).

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-10 21:31 Handle SIGINT in Python Khoo Yit Phang
  2012-01-10 21:47 ` Doug Evans
@ 2012-01-10 21:49 ` Tom Tromey
  2012-01-11 21:15 ` Tom Tromey
  2 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2012-01-10 21:49 UTC (permalink / raw)
  To: Khoo Yit Phang; +Cc: gdb-patches

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

Yit> I've written a patch to GDB's Python support to allow SIGINT to
Yit> interrupt a running script to address the bug
Yit> http://sourceware.org/bugzilla/show_bug.cgi?id=13265 (I've attached my
Yit> patch to that page).

Thanks.

The best way to submit is to follow the contribution instructions:

http://sourceware.org/gdb/contribute/

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-10 21:47 ` Doug Evans
@ 2012-01-10 22:09   ` Khoo Yit Phang
  2012-01-11 20:59   ` Tom Tromey
  1 sibling, 0 replies; 18+ messages in thread
From: Khoo Yit Phang @ 2012-01-10 22:09 UTC (permalink / raw)
  To: Doug Evans; +Cc: Khoo Yit Phang, gdb-patches

Hi,

On Jan 10, 2012, at 4:46 PM, Doug Evans wrote:

> On Tue, Jan 10, 2012 at 1:29 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
>> 
>> There's one caveat: if an inferior is running via, e.g., gdb.execute("run"), SIGINT will interrupt the inferior but not the enclosing "python" command. I think it would be better to also interrupt the "python" command, but I'm not sure how to detect this case yet.
> 
> There is value in having the SIGINT *only* affect the inferior.
> It's up to the script to handle the various reasons why the inferior
> may have stopped, and you don't (generally) want to interfere with
> that (by interrupting the script too).

That makes sense, and I believe it's currently possible to detect that from Python via gdb.events.stop. It might be nicer to throw a custom exception (perhaps a subclass of KeyboardInterrupt), though that can be a different patch.

Yit
January 10, 2012

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-10 21:47 ` Doug Evans
  2012-01-10 22:09   ` Khoo Yit Phang
@ 2012-01-11 20:59   ` Tom Tromey
  2012-01-11 21:06     ` Paul_Koning
  2012-01-12  0:54     ` Doug Evans
  1 sibling, 2 replies; 18+ messages in thread
From: Tom Tromey @ 2012-01-11 20:59 UTC (permalink / raw)
  To: Doug Evans; +Cc: Khoo Yit Phang, gdb-patches

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> There is value in having the SIGINT *only* affect the inferior.
Doug> It's up to the script to handle the various reasons why the inferior
Doug> may have stopped, and you don't (generally) want to interfere with
Doug> that (by interrupting the script too).

I think Python code should have to request something like this
specially.  The scripting case is less usual than the interactive
debugging (perhaps with some Python helper code) case.  I think it would
be weird for the behavior the user sees, by default, to depend on
whether Python or GDB code was active at the moment of C-c.

That is, a C-c when processing a 'python' command in a breakpoint's
'commands' list should have the same general effect as if we were
processing any other command.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: Handle SIGINT in Python
  2012-01-11 20:59   ` Tom Tromey
@ 2012-01-11 21:06     ` Paul_Koning
  2012-01-11 21:23       ` Tom Tromey
  2012-01-12  0:54     ` Doug Evans
  1 sibling, 1 reply; 18+ messages in thread
From: Paul_Koning @ 2012-01-11 21:06 UTC (permalink / raw)
  To: tromey, dje; +Cc: khooyp, gdb-patches

I think the way to make this work is that a C-c in Python is a "KeyboardInterrupt" Python exception.  That can be caught in Python but rarely is.  If not, it would unwind to the interpreter.  A normal Python interpreter would print the default traceback for that exception, exactly as it does for any other exception, it doesn't give interrupt special treatment.  

To get what you're asking (C-c work like in non-Python settings in GDB) all that would be needed is for the interpreter top level to check for specifically the KeyboardInterrupt exception and give that to the GDB C-c handler, rather than printing the default exception traceback.  It would still do that for other (non-interrupt) exceptions.

	paul

-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Tom Tromey
Sent: Wednesday, January 11, 2012 3:56 PM
To: Doug Evans
Cc: Khoo Yit Phang; gdb-patches@sourceware.org
Subject: Re: Handle SIGINT in Python

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> There is value in having the SIGINT *only* affect the inferior.
Doug> It's up to the script to handle the various reasons why the 
Doug> inferior may have stopped, and you don't (generally) want to 
Doug> interfere with that (by interrupting the script too).

I think Python code should have to request something like this specially.  The scripting case is less usual than the interactive debugging (perhaps with some Python helper code) case.  I think it would be weird for the behavior the user sees, by default, to depend on whether Python or GDB code was active at the moment of C-c.

That is, a C-c when processing a 'python' command in a breakpoint's 'commands' list should have the same general effect as if we were processing any other command.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-10 21:31 Handle SIGINT in Python Khoo Yit Phang
  2012-01-10 21:47 ` Doug Evans
  2012-01-10 21:49 ` Tom Tromey
@ 2012-01-11 21:15 ` Tom Tromey
  2012-01-11 21:49   ` Khoo Yit Phang
  2 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2012-01-11 21:15 UTC (permalink / raw)
  To: Khoo Yit Phang; +Cc: gdb-patches

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

Yit> I've written a patch to GDB's Python support to allow SIGINT to
Yit> interrupt a running script to address the bug
Yit> http://sourceware.org/bugzilla/show_bug.cgi?id=13265 (I've attached my
Yit> patch to that page).

See my other note about how to send the patch; and my other other note
about copyright assignments.

I don't understand why this code doesn't just pass the old signal
handler as an argument to the cleanup function.  I think that would be
more readable.

What happens when the Python code calls back into gdb?
We do this in many places in gdb/python/*.c.
It seems to me that you'd have to temporarily push gdb's SIGINT handler
again.

Could we possibly solve this problem without constantly resetting the
SIGINT handler?  Maybe via a combination of a global flag plus a call
into Python from handle_sigint?

Our python->gdb exception story is not super.  And, we lose information
in the round trip.  This might (or might not...) be a prerequisite to
solving this problem.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-11 21:06     ` Paul_Koning
@ 2012-01-11 21:23       ` Tom Tromey
  0 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2012-01-11 21:23 UTC (permalink / raw)
  To: Paul_Koning; +Cc: dje, khooyp, gdb-patches

>>>>> "Paul" ==   <Paul_Koning@Dell.com> writes:

Paul> To get what you're asking (C-c work like in non-Python settings in
Paul> GDB) all that would be needed is for the interpreter top level to
Paul> check for specifically the KeyboardInterrupt exception and give that
Paul> to the GDB C-c handler, rather than printing the default exception
Paul> traceback.  It would still do that for other (non-interrupt)
Paul> exceptions.

I think there are basically 2 cases to consider: does the SIGINT arrive
in Python code, or in GDB code?

In GDB code, the SIGINT handler should be handle_sigint.  (AFAIK --
there may be other situations, and I'm not familiar with all of them).
This sets quit_flag.  Then, appropriate spots in GDB invoke QUIT, which
checks this flag and calls 'fatal' if it is set.  fatal just throws a
RETURN_QUIT exception.

In Python code, <mumble> is the SIGINT handler.  I believe it calls
PyErr_SetInterrupt.  Then Python does something similar: it checks this
flag periodically and turns it into a KeyboardInterrupt exception.


We are already pretty good at converting GDB exceptions to Python
exceptions.  gdbpy_convert_exception handles the RETURN_QUIT ->
KeyboardInterrupt case already.

We are pretty bad at converting Python exceptions to GDB exceptions.
See:
http://sourceware.org/bugzilla/show_bug.cgi?id=12174
This is the problem where we lose information in exception round-trips.

So, one idea would be to make this better: fix the round-trip problem,
and convert KeyboardInterrupt to RETURN_QUIT, then find all the
Python->GDB boundaries and make them deal with this as appropriate.

Perhaps we could also just have a single SIGINT handler that can handle
all cases.  I am not sure.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-11 21:15 ` Tom Tromey
@ 2012-01-11 21:49   ` Khoo Yit Phang
  2012-01-11 22:46     ` Khoo Yit Phang
  2012-01-20 21:40     ` Tom Tromey
  0 siblings, 2 replies; 18+ messages in thread
From: Khoo Yit Phang @ 2012-01-11 21:49 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Khoo Yit Phang, gdb-patches

Hi,

On Jan 11, 2012, at 4:06 PM, Tom Tromey wrote:

>>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:
> 
> Yit> I've written a patch to GDB's Python support to allow SIGINT to
> Yit> interrupt a running script to address the bug
> Yit> http://sourceware.org/bugzilla/show_bug.cgi?id=13265 (I've attached my
> Yit> patch to that page).
> 
> See my other note about how to send the patch; and my other other note
> about copyright assignments.

I'll attach a patch in a moment.

> I don't understand why this code doesn't just pass the old signal
> handler as an argument to the cleanup function.  I think that would be
> more readable.
> What happens when the Python code calls back into gdb?
> We do this in many places in gdb/python/*.c.
> It seems to me that you'd have to temporarily push gdb's SIGINT handler
> again.

It is not the old signal handler that's being restored. My implementation handles the following sequence of events:

... SIGINT is handle_sigint_orig ...

(gdb) python

... SIGINT is python_handle_sigint

>>>  gdb.execute("something that changes the SIGINT handler to handle_sigint_new")

... python_handle_sigint is suspended and SIGINT is restored to handle_sigint_orig 
... gdb.execute runs and changes SIGINT is updated to handle_sigint_new
... python_handle_sigint is resumed, but python_saved_sigint_handler is now handle_sigint_new

>>> ^D

... SIGINT is handle_sigint_new, *not* handle_sigint_orig

(gdb)


> Could we possibly solve this problem without constantly resetting the
> SIGINT handler?  Maybe via a combination of a global flag plus a call
> into Python from handle_sigint?

It is possible, I just need a way to call PySet_Interrupt. But it seems to be that it would require adding hooks to events-top.c and, which seems like a separate project.

> Our python->gdb exception story is not super.  And, we lose information
> in the round trip.  This might (or might not...) be a prerequisite to
> solving this problem.

I don't think it's a problem, unless for nested calls to python like "py gdb.execute('py ...')".

Yit
January 11, 2012

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-11 21:49   ` Khoo Yit Phang
@ 2012-01-11 22:46     ` Khoo Yit Phang
  2012-01-20 21:40     ` Tom Tromey
  1 sibling, 0 replies; 18+ messages in thread
From: Khoo Yit Phang @ 2012-01-11 22:46 UTC (permalink / raw)
  To: Khoo Yit Phang; +Cc: Tom Tromey, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

Hi,

On Jan 11, 2012, at 4:45 PM, Khoo Yit Phang wrote:
> 
> On Jan 11, 2012, at 4:06 PM, Tom Tromey wrote:
> 
>>>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:
>> 
>> Yit> I've written a patch to GDB's Python support to allow SIGINT to
>> Yit> interrupt a running script to address the bug
>> Yit> http://sourceware.org/bugzilla/show_bug.cgi?id=13265 (I've attached my
>> Yit> patch to that page).
>> 
>> See my other note about how to send the patch;
> 
> I'll attach a patch in a moment.

Here it is.

Yit
January 11, 2012


[-- Attachment #2: python-handle-sigint.diff --]
[-- Type: application/octet-stream, Size: 3816 bytes --]

Allow SIGINT to interrupt the "python" command.

gdb/Changelog:

2012-01-11  Khoo Yit Phang <khooyp@cs.umd.edu>

	Allow SIGINT to interrupt the "python" command.
	* python/python-internal.h (gdbpy_suspend_sigint_handler): New
	prototype>
	* python/python.c (gdbpy_saved_sigint_handler): New variable.
	(gdbpy_handle_sigint): New function.
	(gdbpy_restore_sigint_handler): Ditto.
	(gdbpy_install_sigint_handler): Ditto.
	(gdbpy_resume_sigint_handler): Ditto.
	(gdbpy_suspend_sigint_handler): Ditto.
	(restore_python_env): Call gdbpy_install_sigint_handler.
	(ensure_python_env): Call gdbpy_restore_sigint_handler.
	(python_command): Move call to ensure_python_env closer to
	Python entry.
	(execute_gdb_command): Call gdbpy_suspend_sigint_handler.

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -102,8 +102,11 @@
 #include "command.h"
 #include "breakpoint.h"
 
+#include "defs.h"
 #include "exceptions.h"
 
+struct cleanup *gdbpy_suspend_sigint_handler (void);
+
 enum gdbpy_iter_kind { iter_keys, iter_values, iter_items };
 
 struct block;
diff --git a/gdb/python/python.c b/gdb/python/python.c
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -97,6 +97,51 @@
 struct gdbarch *python_gdbarch;
 const struct language_defn *python_language;
 
+/* When entering Python, install a Python-specific SIGINT handler;
+   restore the original handler upon exit. */
+
+static sig_t gdbpy_saved_sigint_handler;
+
+static void
+gdbpy_handle_sigint (int sig)
+{
+  PyErr_SetInterrupt ();
+}
+
+static void
+gdbpy_restore_sigint_handler (void)
+{
+  signal (SIGINT, gdbpy_saved_sigint_handler);
+}
+
+static void
+gdbpy_install_sigint_handler (void)
+{
+  gdbpy_saved_sigint_handler = signal (SIGINT, gdbpy_handle_sigint);
+}
+
+/* When calling GDB functions from Python, temporarily suspend the
+   Python-specific SIGINT handler and restore the original handler
+   (currently only called by gdb.execute since it's most likely to
+   take a long time).
+
+   When resuming (upon cleanup), note that the saved SIGINT handler
+   is updated to the handler installed at that time, to propagate
+   change to the SIGINT handler down the call stack. */
+
+static void
+gdbpy_resume_sigint_handler (void *s)
+{
+  gdbpy_install_sigint_handler ();
+}
+
+struct cleanup *
+gdbpy_suspend_sigint_handler (void)
+{
+  gdbpy_restore_sigint_handler ();
+  return make_cleanup (gdbpy_resume_sigint_handler, NULL);
+}
+
 /* Restore global language and architecture and Python GIL state
    when leaving the Python interpreter.  */
 
@@ -123,6 +168,8 @@
 
   PyErr_Restore (env->error_type, env->error_value, env->error_traceback);
 
+  gdbpy_restore_sigint_handler ();
+
   PyGILState_Release (env->state);
   python_gdbarch = env->gdbarch;
   python_language = env->language;
@@ -142,6 +189,8 @@
   env->gdbarch = python_gdbarch;
   env->language = python_language;
 
+  gdbpy_install_sigint_handler ();
+
   python_gdbarch = gdbarch;
   python_language = language;
 
@@ -261,15 +310,15 @@
 {
   struct cleanup *cleanup;
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
-
-  make_cleanup_restore_integer (&interpreter_async);
+  cleanup = make_cleanup_restore_integer (&interpreter_async);
   interpreter_async = 0;
 
   while (arg && *arg && isspace (*arg))
     ++arg;
   if (arg && *arg)
     {
+      ensure_python_env (get_current_arch (), current_language);
+
       if (PyRun_SimpleString (arg))
 	error (_("Error while executing Python code."));
     }
@@ -442,6 +491,7 @@
       char *copy = xstrdup (arg);
       struct cleanup *cleanup = make_cleanup (xfree, copy);
 
+      gdbpy_suspend_sigint_handler ();
       make_cleanup_restore_integer (&interpreter_async);
       interpreter_async = 0;
 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-11 20:59   ` Tom Tromey
  2012-01-11 21:06     ` Paul_Koning
@ 2012-01-12  0:54     ` Doug Evans
  2012-01-12 15:52       ` Kevin Pouget
  1 sibling, 1 reply; 18+ messages in thread
From: Doug Evans @ 2012-01-12  0:54 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Khoo Yit Phang, gdb-patches

On Wed, Jan 11, 2012 at 12:55 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> There is value in having the SIGINT *only* affect the inferior.
> Doug> It's up to the script to handle the various reasons why the inferior
> Doug> may have stopped, and you don't (generally) want to interfere with
> Doug> that (by interrupting the script too).
>
> I think Python code should have to request something like this
> specially.  The scripting case is less usual than the interactive
> debugging (perhaps with some Python helper code) case.  I think it would
> be weird for the behavior the user sees, by default, to depend on
> whether Python or GDB code was active at the moment of C-c.

I'm not sure we're talking about the same thing.
I wasn't suggesting the behaviour be different depending on whether
GDB or Python was running.
[For reference sake, to make sure we're on the same page, my comment
is solely for the case where the inferior is launched by gdb, the
inferior is running, has the terminal, and will get the SIGINT.]

> That is, a C-c when processing a 'python' command in a breakpoint's
> 'commands' list should have the same general effect as if we were
> processing any other command.

PTAL.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-12  0:54     ` Doug Evans
@ 2012-01-12 15:52       ` Kevin Pouget
  2012-01-12 16:48         ` Paul_Koning
  0 siblings, 1 reply; 18+ messages in thread
From: Kevin Pouget @ 2012-01-12 15:52 UTC (permalink / raw)
  To: Khoo Yit Phang; +Cc: Tom Tromey, gdb-patches, Doug Evans

On Thu, Jan 12, 2012 at 1:17 AM, Doug Evans <dje@google.com> wrote:
>
> On Wed, Jan 11, 2012 at 12:55 PM, Tom Tromey <tromey@redhat.com> wrote:
> >>>>>> "Doug" == Doug Evans <dje@google.com> writes:
> >
> > Doug> There is value in having the SIGINT *only* affect the inferior.
> > Doug> It's up to the script to handle the various reasons why the inferior
> > Doug> may have stopped, and you don't (generally) want to interfere with
> > Doug> that (by interrupting the script too).
> >
> > I think Python code should have to request something like this
> > specially.  The scripting case is less usual than the interactive
> > debugging (perhaps with some Python helper code) case.  I think it would
> > be weird for the behavior the user sees, by default, to depend on
> > whether Python or GDB code was active at the moment of C-c.
>
> I'm not sure we're talking about the same thing.
> I wasn't suggesting the behaviour be different depending on whether
> GDB or Python was running.
> [For reference sake, to make sure we're on the same page, my comment
> is solely for the case where the inferior is launched by gdb, the
> inferior is running, has the terminal, and will get the SIGINT.]
>
> > That is, a C-c when processing a 'python' command in a breakpoint's
> > 'commands' list should have the same general effect as if we were
> > processing any other command.
>
> PTAL.

Hello,

just my 2 cents to the discussion.
[I'm not sure I could follow all the arguments, so my comments are
based on Khoo's patch]

+static void
+gdbpy_handle_sigint (int sig)
+{
+  PyErr_SetInterrupt ();
+}

I would be quite afraid if GDB could throw an
KeyboardInterruptException in any call to its API, there wouldn't be
any way to ensure internal consistency inside the script, except with
try_catch surrounding every GDB function call ...

In http://sourceware.org/bugzilla/show_bug.cgi?id=12615 I suggested
using something similar to what already exist in GDB:

> #define QUIT { \
>   if (quit_flag) quit (); \
>   if (deprecated_interactive_hook) deprecated_interactive_hook (); \
> }

and:

> /* Nonzero means a quit has been requested.  */
> int quit_flag;
> int immediate_quit;

that is, setting a Python-reachable flag and let the script decides what to do.

`remote.c' mechanism

> /* Signal handler for SIGINT, while the target is executing.  */
> static void
>
> handle_remote_sigint (int sig)
> {
>  signal (sig, handle_remote_sigint_twice);
>  ...

> /* Signal handler for SIGINT, installed after SIGINT has already been
>   sent once.  It will take effect the second time that the user sends
>   a ^C.  */
>  static void
>  handle_remote_sigint_twice (int sig)

could also be used to actually send the KeyboardInterruptException if
C^c is hit twice.


Kevin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: Handle SIGINT in Python
  2012-01-12 15:52       ` Kevin Pouget
@ 2012-01-12 16:48         ` Paul_Koning
  2012-01-13 10:55           ` Kevin Pouget
  0 siblings, 1 reply; 18+ messages in thread
From: Paul_Koning @ 2012-01-12 16:48 UTC (permalink / raw)
  To: kevin.pouget, khooyp; +Cc: tromey, gdb-patches, dje

>just my 2 cents to the discussion.
>[I'm not sure I could follow all the arguments, so my comments are based on Khoo's patch]
>
>+static void
>+gdbpy_handle_sigint (int sig)
>+{
>+  PyErr_SetInterrupt ();
>+}
>
>I would be quite afraid if GDB could throw an KeyboardInterruptException in any call to its API, there wouldn't be any way to ensure internal consistency inside the script, except with try_catch surrounding every GDB function call ...

Of course, but that is exactly what you want.  In any Python script, if you need to handle interrupts for proper operation, you wrap the critical section in a try..except block.  If not, then you don't.  There is nothing new or different here in the GDB case.  Any Python statement can throw a KeyboardException.

The only difference is in what happens when you get to the top of a Python script with a KeyboardInterrupt exception and no one is catching it.  In an interactive session (python shell) you get a backtrace.  I think in a Python interactive session inside GDB ("python" command with no arguments) the same should be true.  In a script invocation from the command shell (outside GDB) you'd also get a traceback in this case.  In GDB, you probably should not, as was discussed earlier; in that case, the interrupt should be passed up to the GDB common machinery to handle exactly as it would handle an interrupt for any other command, without a Python style traceback printout.

	paul

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-12 16:48         ` Paul_Koning
@ 2012-01-13 10:55           ` Kevin Pouget
  2012-01-13 12:11             ` Paul_Koning
  0 siblings, 1 reply; 18+ messages in thread
From: Kevin Pouget @ 2012-01-13 10:55 UTC (permalink / raw)
  To: Paul_Koning, khooyp; +Cc: tromey, gdb-patches, dje

On Thu, Jan 12, 2012 at 5:47 PM,  <Paul_Koning@dell.com> wrote:
>>just my 2 cents to the discussion.
>>[I'm not sure I could follow all the arguments, so my comments are based on Khoo's patch]
>>
>>+static void
>>+gdbpy_handle_sigint (int sig)
>>+{
>>+  PyErr_SetInterrupt ();
>>+}
>>

>
> The only difference is in what happens when you get to the top of a Python script with a KeyboardInterrupt exception and no one is catching it.  In an interactive session (python shell) you get a backtrace.  I think in a Python interactive session inside GDB ("python" command with no arguments) the same should be true.  In a script invocation from the command shell (outside GDB) you'd also get a traceback in this case.  In GDB, you probably should not, as was discussed earlier; in that case, the interrupt should be passed up to the GDB common machinery to handle exactly as it would handle an interrupt for any other command, without a Python style traceback printout.

On Thu, Jan 12, 2012 at 4:35 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> Your concern does not arise: PyErr_SetInterrupt does not raise any exception, it simply sets a flag which Python checks every now and then in the
> interpreter loop. The same thing happens for any Python exception that is raised in C: you would set an exception via PyErr_SetString(exception,
> message) or similar, and return NULL or -1 to the interpreter.


Thanks for the clarifications, but I'm still not unsure about how to
build a reliable application on top of GDB with this kind of SIGINT
handling.
I'm not an expert on Python programming nor on 'reliable application
design' (and I'm used to C programming) so please bear with me if
there are some flaws in my arguments, I'm just trying to figure out
how this patch would influence my code:)


> Any Python statement can throw a KeyboardException.
That could make sense in a stateless / lightweight application, but in
[my] GDB-base Python application, I can't see how this can remain the
default behavior ... I'm a bit stuck with the FLAG handling I
described in the previous mail, 1 C^c sets a flag, 2 C^c raises an
exception (and almost kills the app), but I can't see any other way to
safely handle this SIGINT.

(or I can implement the flag mechanism by myself, as in
http://stackoverflow.com/a/4205386/341106:
> import signal
> def signal_handler(signal, frame):
>    print 'You pressed Ctrl+C!'
>    # what ever ...
> signal.signal(signal.SIGINT, signal_handler)

but I guess I would still have to surround GDB function calls with
KeyboardInterrupt handling ...)


Cordially,

Kevin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: Handle SIGINT in Python
  2012-01-13 10:55           ` Kevin Pouget
@ 2012-01-13 12:11             ` Paul_Koning
  0 siblings, 0 replies; 18+ messages in thread
From: Paul_Koning @ 2012-01-13 12:11 UTC (permalink / raw)
  To: kevin.pouget, khooyp; +Cc: tromey, gdb-patches, dje

>...
>> Any Python statement can throw a KeyboardException.
>That could make sense in a stateless / lightweight application, but in
>[my] GDB-base Python application, I can't see how this can remain the
>default behavior ... 

But it is in every Python program; if you program in Python it's part of what you do.  Python, unlike many other programming languages, treats exceptions as a major mechanism that's ingrained in everything.  It's not missing (as in C) or more of an afterthought (as in C++).  I think that having GDB/Python work differently would make it feel quite unPythonic.

	paul 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-11 21:49   ` Khoo Yit Phang
  2012-01-11 22:46     ` Khoo Yit Phang
@ 2012-01-20 21:40     ` Tom Tromey
  2012-01-22 16:36       ` Khoo Yit Phang
  1 sibling, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2012-01-20 21:40 UTC (permalink / raw)
  To: Khoo Yit Phang; +Cc: gdb-patches

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

Tom> Could we possibly solve this problem without constantly resetting the
Tom> SIGINT handler?  Maybe via a combination of a global flag plus a call
Tom> into Python from handle_sigint?

Yit> It is possible, I just need a way to call PySet_Interrupt. But it
Yit> seems to be that it would require adding hooks to events-top.c and,
Yit> which seems like a separate project.

I think of it more as a different implementation of the feature...

Tom> Our python->gdb exception story is not super.  And, we lose information
Tom> in the round trip.  This might (or might not...) be a prerequisite to
Tom> solving this problem.

Yit> I don't think it's a problem, unless for nested calls to python like
Yit> "py gdb.execute('py ...')".

You can't fully predict what gdb actions will cause Python code to run.

Also, Python frequently calls into gdb.  Any place in gdb that invokes
QUIT could potentially see a C-c.  However, IIUC, with your patch these
QUITs will be inactive if there is Python code up the stack -- but this
means that some slow things in gdb will be uninterruptible.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-20 21:40     ` Tom Tromey
@ 2012-01-22 16:36       ` Khoo Yit Phang
  2012-01-22 20:54         ` Khoo Yit Phang
  0 siblings, 1 reply; 18+ messages in thread
From: Khoo Yit Phang @ 2012-01-22 16:36 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Khoo Yit Phang, gdb-patches

Hi,

On Jan 20, 2012, at 4:33 PM, Tom Tromey wrote:

>>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:
> 
> Tom> Could we possibly solve this problem without constantly resetting the
> Tom> SIGINT handler?  Maybe via a combination of a global flag plus a call
> Tom> into Python from handle_sigint?
> 
> Yit> It is possible, I just need a way to call PySet_Interrupt. But it
> Yit> seems to be that it would require adding hooks to events-top.c and,
> Yit> which seems like a separate project.
> 
> I think of it more as a different implementation of the feature...

In the long run, I do think that changing events-top.c is better, but I've to figure out how it should look like, for Python and other clients.

> Tom> Our python->gdb exception story is not super.  And, we lose information
> Tom> in the round trip.  This might (or might not...) be a prerequisite to
> Tom> solving this problem.
> 
> Yit> I don't think it's a problem, unless for nested calls to python like
> Yit> "py gdb.execute('py ...')".
> 
> You can't fully predict what gdb actions will cause Python code to run.

Actually, I don't think I understand what you mean. Can you give an example? (Is it related to scenario 1 below?)

> Also, Python frequently calls into gdb.  Any place in gdb that invokes
> QUIT could potentially see a C-c.  However, IIUC, with your patch these
> QUITs will be inactive if there is Python code up the stack -- but this
> means that some slow things in gdb will be uninterruptible.

I think there are two scenarios here:

1. When the call stack is GDB -> Python, Ctrl-C will only interrupt the Python part (unless there's special handling in the GDB part to treat KeyboardInterrupt). You would have to press Ctrl-C again to interrupt the GDB part immediately after. I wondered whether to have Ctrl-C interrupt the entire stack; it's fairly easy to do, since every Python entry is wrapped by ensure_python_env (just check for KeyboardInterrupt during cleanup and raise QUIT).

Though, there was some debate earlier in the mailing list on something related: whether Ctrl-C during gdb.execute should interrupt Python too. The concern was that there may be legitimate reasons to interrupt only part of the call stack, and the decision should depend on the particulars of the task.

2. When the call stack is GDB -> Only gdb.execute is interruptible now, since that's the only place I've suspended Python's interrupt handler. There may be other Python methods that call into GDB that is slow that could use the same treatment.

Yit
January 22, 2012

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Handle SIGINT in Python
  2012-01-22 16:36       ` Khoo Yit Phang
@ 2012-01-22 20:54         ` Khoo Yit Phang
  0 siblings, 0 replies; 18+ messages in thread
From: Khoo Yit Phang @ 2012-01-22 20:54 UTC (permalink / raw)
  To: Khoo Yit Phang; +Cc: Tom Tromey, gdb-patches

Hi,

On Jan 22, 2012, at 11:32 AM, Khoo Yit Phang wrote:
> 
> 2. When the call stack is GDB -> Only gdb.execute is interruptible now, since that's the only place I've suspended Python's interrupt handler. There may be other Python methods that call into GDB that is slow that could use the same treatment.

Whoops, that should be "When the call stack is GDB -> Python -> GDB, ...".

Yit
January 22, 2012

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-01-22 16:57 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-10 21:31 Handle SIGINT in Python Khoo Yit Phang
2012-01-10 21:47 ` Doug Evans
2012-01-10 22:09   ` Khoo Yit Phang
2012-01-11 20:59   ` Tom Tromey
2012-01-11 21:06     ` Paul_Koning
2012-01-11 21:23       ` Tom Tromey
2012-01-12  0:54     ` Doug Evans
2012-01-12 15:52       ` Kevin Pouget
2012-01-12 16:48         ` Paul_Koning
2012-01-13 10:55           ` Kevin Pouget
2012-01-13 12:11             ` Paul_Koning
2012-01-10 21:49 ` Tom Tromey
2012-01-11 21:15 ` Tom Tromey
2012-01-11 21:49   ` Khoo Yit Phang
2012-01-11 22:46     ` Khoo Yit Phang
2012-01-20 21:40     ` Tom Tromey
2012-01-22 16:36       ` Khoo Yit Phang
2012-01-22 20:54         ` Khoo Yit Phang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).