Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

[Doug Evans]

[+ guile-user, in case they have any thoughts on Guile SIGINT handling]

On Mon, Dec 23, 2013 at 1:57 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <xdje42@gmail.com> writes:
>
> Doug> +void
> Doug> +clear_quit_flag (void)
> Doug> +{
> Doug> +  int i;
> Doug> +  const struct extension_language_defn *extlang;
> Doug> +
> Doug> +  ALL_ENABLED_EXTENSION_LANGUAGES (i, extlang)
> Doug> +    {
> Doug> +      if (extlang->ops->clear_quit_flag != NULL)
> Doug> + extlang->ops->clear_quit_flag (extlang);
> Doug> +    }
> Doug> +
> Doug> +  quit_flag = 0;
> Doug> +}
>
> I don't think this will work properly.  It seems to me that a given
> interrupt may be processed multiple times -- once by each extension
> language.

Guile doesn't provide the same hooks that Python does for SIGINT
(PyErr_SetInterrupt, et.al.) so I expect this part to need some work.

> The way it works right now, if Python handles the "quit", then Python
> clears the flag -- which also clears gdb's notion of the flag, because
> the two are identical.
>
> With the above it seems that Python could clear its flag -- but leave
> other extension languages unaware that the interrupt was handled.  So, a
> subsequent call into Guile will presumably erroneously throw an
> exception.

We carefully manage entry and exit to/from extension languages.
It should be possible to manage SIGINT across these boundaries.
v3 coming up, but I'm leaving this part to v4.

Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

[Ludovic Courtès]

Doug Evans <xdje42@gmail.com> skribis:

> On Mon, Dec 23, 2013 at 1:57 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> "Doug" == Doug Evans <xdje42@gmail.com> writes:
>>
>> Doug> +void
>> Doug> +clear_quit_flag (void)
>> Doug> +{
>> Doug> +  int i;
>> Doug> +  const struct extension_language_defn *extlang;
>> Doug> +
>> Doug> +  ALL_ENABLED_EXTENSION_LANGUAGES (i, extlang)
>> Doug> +    {
>> Doug> +      if (extlang->ops->clear_quit_flag != NULL)
>> Doug> + extlang->ops->clear_quit_flag (extlang);
>> Doug> +    }
>> Doug> +
>> Doug> +  quit_flag = 0;
>> Doug> +}
>>
>> I don't think this will work properly.  It seems to me that a given
>> interrupt may be processed multiple times -- once by each extension
>> language.
>
> Guile doesn't provide the same hooks that Python does for SIGINT
> (PyErr_SetInterrupt, et.al.) so I expect this part to need some work.
>
>> The way it works right now, if Python handles the "quit", then Python
>> clears the flag -- which also clears gdb's notion of the flag, because
>> the two are identical.
>>
>> With the above it seems that Python could clear its flag -- but leave
>> other extension languages unaware that the interrupt was handled.  So, a
>> subsequent call into Guile will presumably erroneously throw an
>> exception.
>
> We carefully manage entry and exit to/from extension languages.
> It should be possible to manage SIGINT across these boundaries.
> v3 coming up, but I'm leaving this part to v4.

I’m not sure I understand the problem.

What part of the code calls signal(2) or sigaction(2) in the end?  There
can only be one SIGINT handler in the process anyway, so I guess it
has to be installed by GDB (the extension-language-independent part), no?

What are extension languages typically expected to do when the ‘quit’
flag is set?

(And happy new year, where applicable! ;-))

Ludo’.

Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

[Tom Tromey]

>>>>> "Ludovic" == Ludovic Courtès <ludo@gnu.org> writes:

Ludovic> I’m not sure I understand the problem.

Ludovic> What are extension languages typically expected to do when the ‘quit’
Ludovic> flag is set?

The basic idea is that if some extension code is running, then C-c ought
to interrupt that code in the way expected by programmers writing code
in that language.

Python provides a kind of low-level API to its equivalent of Guile's
SCM_TICK and async stuff.  So the approach we took in gdb was to unify
gdb's implementation with Python's, when Python is enabled.  This
ensures that a SIGINT delivery is handled a single time -- by Python if
Python code is running, and by gdb if gdb code is running.

One approach for multiple extension languages might be to notice when
switching languages and move the bit.  However I didn't see any way to
do this in the Guile API.  In Python it can be accomplished with
PyErr_SetInterrupt and PyOS_InterruptOccurred.

Tom

Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

[Ludovic Courtès]

Tom Tromey <tromey@redhat.com> skribis:

>>>>>> "Ludovic" == Ludovic Courtès <ludo@gnu.org> writes:
>
> Ludovic> I’m not sure I understand the problem.
>
> Ludovic> What are extension languages typically expected to do when the ‘quit’
> Ludovic> flag is set?
>
> The basic idea is that if some extension code is running, then C-c ought
> to interrupt that code in the way expected by programmers writing code
> in that language.
>
> Python provides a kind of low-level API to its equivalent of Guile's
> SCM_TICK and async stuff.  So the approach we took in gdb was to unify
> gdb's implementation with Python's, when Python is enabled.  This
> ensures that a SIGINT delivery is handled a single time -- by Python if
> Python code is running, and by gdb if gdb code is running.

So do I get it right that it’s GDB that does signal(SIGINT, ...), and
its handler just calls PyOS_InterruptOccurred (or so) if Python code
happens to be running?

> One approach for multiple extension languages might be to notice when
> switching languages and move the bit.  However I didn't see any way to
> do this in the Guile API.  In Python it can be accomplished with
> PyErr_SetInterrupt and PyOS_InterruptOccurred.

Would it work, upon SIGINT, to do something like:

  (system-async-mark (lambda ()
                       (throw 'system-error ... EINTR)))

That would eventually raise an exception in Scheme code.

Ludo’.

Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

[Tom Tromey]

>>>>> "Ludovic" == Ludovic Courtès <ludo@gnu.org> writes:

Ludovic> So do I get it right that it’s GDB that does signal(SIGINT, ...), and
Ludovic> its handler just calls PyOS_InterruptOccurred (or so) if Python code
Ludovic> happens to be running?

Yeah.

>> One approach for multiple extension languages might be to notice when
>> switching languages and move the bit.  However I didn't see any way to
>> do this in the Guile API.  In Python it can be accomplished with
>> PyErr_SetInterrupt and PyOS_InterruptOccurred.

Ludovic> Would it work, upon SIGINT, to do something like:
Ludovic>   (system-async-mark (lambda ()
Ludovic>                        (throw 'system-error ... EINTR)))
Ludovic> That would eventually raise an exception in Scheme code.

According to the Guile docs one must use scm_sigaction, since
scm_system_async_mark is not async-signal-safe.

However scm_sigaction takes over the signal.  I don't think that will
work ok.  Also this idea doesn't address how it would interact with the
gdb core or with Python.

Tom

Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

[Ludovic Courtès]

Tom Tromey <tromey@redhat.com> skribis:

> Ludovic> Would it work, upon SIGINT, to do something like:
> Ludovic>   (system-async-mark (lambda ()
> Ludovic>                        (throw 'system-error ... EINTR)))
> Ludovic> That would eventually raise an exception in Scheme code.
>
> According to the Guile docs one must use scm_sigaction, since
> scm_system_async_mark is not async-signal-safe.

Oh, right.  This could be worked around by having a separate signal
delivery thread (which is what Guile currently does internally), but
that’s not so great.

I guess this is another limitation of Guile’s current signal handling
strategy, and something we should fix.

Ludo’.

Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

[Tom Tromey]

>>>>> "Ludovic" == Ludovic Courtès <ludo@gnu.org> writes:

Ludovic> I guess this is another limitation of Guile’s current signal handling
Ludovic> strategy, and something we should fix.

I think it seems reasonable for Guile itself, but yeah, it makes it
harder to integrate into gdb.  I was pleasantly surprised that Python's
approach was similar enough to gdb's that we could easily integrate the
two.

The Python approach -- basically just setting a per-signal flag that is
checked in the Python equivalent of SCM_TICK -- might be nice for Guile;
however Guile has better threading support, so some additional work may
be required.

Also, at least for gdb it may make sense to mask SIGINT and SIGCHLD in
new Guile threads, if that is possible.

Tom

Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

[Tom Tromey]

>>>>> "Ludovic" == Ludovic Courtès <ludo@gnu.org> writes:

Ludovic> I guess this is another limitation of Guile’s current signal handling
Ludovic> strategy, and something we should fix.

FWIW I think it would be sufficient for gdb if scm_system_async_mark, or
something like it, could be invoked from a signal handler.  Then a
SIGINT in gdb could install an async callback that later checks gdb's
quit flag.

Tom

Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

[Ludovic Courtès]

Tom Tromey <tromey@redhat.com> skribis:

>>>>>> "Ludovic" == Ludovic Courtès <ludo@gnu.org> writes:
>
> Ludovic> I guess this is another limitation of Guile’s current signal handling
> Ludovic> strategy, and something we should fix.
>
> FWIW I think it would be sufficient for gdb if scm_system_async_mark, or
> something like it, could be invoked from a signal handler.  Then a
> SIGINT in gdb could install an async callback that later checks gdb's
> quit flag.

Right.  Unfortunately that seems hard to fix in the 2.0 series, but 2.2
is not too far away either.

Thanks,
Ludo’.

Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

[Mark H Weaver]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Ludovic" == Ludovic Courtès <ludo@gnu.org> writes:
>
> Ludovic> I guess this is another limitation of Guile’s current signal handling
> Ludovic> strategy, and something we should fix.
>
> FWIW I think it would be sufficient for gdb if scm_system_async_mark, or
> something like it, could be invoked from a signal handler.  Then a
> SIGINT in gdb could install an async callback that later checks gdb's
> quit flag.

As discussed on IRC, one way to accomplish this with current Guile is
as follows: establish a dedicated thread whose sole job it is to call
'scm_system_async_mark_for_thread' as directed by messages received from
a pipe.  A signal handler can then schedule asyncs by writing messages
to this pipe.

We'll try to come up with an nicer solution at some point.

     Mark

Re: [PATCH v2 02/13] script language API for GDB: extension.[ch]

[Doug Evans]

On Tue, Jan 21, 2014 at 8:21 PM, Mark H Weaver <mhw@netris.org> wrote:
> Tom Tromey <tromey@redhat.com> writes:
>
>>>>>>> "Ludovic" == Ludovic Courtès <ludo@gnu.org> writes:
>>
>> Ludovic> I guess this is another limitation of Guile’s current signal handling
>> Ludovic> strategy, and something we should fix.
>>
>> FWIW I think it would be sufficient for gdb if scm_system_async_mark, or
>> something like it, could be invoked from a signal handler.  Then a
>> SIGINT in gdb could install an async callback that later checks gdb's
>> quit flag.
>
> As discussed on IRC, one way to accomplish this with current Guile is
> as follows: establish a dedicated thread whose sole job it is to call
> 'scm_system_async_mark_for_thread' as directed by messages received from
> a pipe.  A signal handler can then schedule asyncs by writing messages
> to this pipe.
>
> We'll try to come up with an nicer solution at some point.

Righto, and thanks.

I'll implement this as a follow-on to the current patch series.
It'll be mostly just new code, and the current patch series is big enough.