Re: Use of rl_echo_signal_char in _rl_handle_signal

[Andrew Burgess]

Chet Ramey <chet.ramey@case.edu> writes:

> On 6/1/23 6:40 AM, Andrew Burgess wrote:
>> 
>> This email originated as a conversation on the GDB list, which can be
>> found here:
>> 
>>    
>> https://inbox.sourceware.org/gdb-patches/20230518061046.17837-1-tdevries@suse.de/
>> 
>> The problem that was observed was that sometimes during testing GDB
>> would print '^C', but most of the time this would not happen.
>> 
>> The '^C' is printed from rl_echo_signal_char.
>> 
>> What normally happens is that the user types a character, GDB sees the
>> character appear on stdin and calls into readline to process the
>> character.  Once completed readline returns to GDB to await the next
>> event.  In this case the next event is the user typing 'Ctrl+c' which
>> sends SIGINT.  GDB's signal handler catches this and does its thing.
>
> This works fine if the application supplies its own signal handlers, like
> GDB does. There are lots of applications that use readline that don't, but
> would still like to see the signal characters. These are usually the ones
> that set rl_persistent_signal_handlers, but not always. Making the change
> you suggest woul break those apps in an incompatible way, with no way to
> get their desired behavior.

Thanks for pointing that out, I was not aware of this setting.

>
>> The documentation for rl_echo_signal_char says:
>> 
>>       If an application wishes to install its own signal handlers, but
>>       still have readline display characters that generate signals,
>>       calling this function with SIG set to 'SIGINT', 'SIGQUIT', or
>>       'SIGTSTP' will display the character generating that signal.
>> 
>> I would also make the assumption that, when readline is used in callback
>> mode, the application spends most of its time with the applications
>> signal handlers installed, and only installs readline's handlers
>> occasionally.
>
> This is a good assumption for GDB, but not for other applications. Several
> interactive python applications, for instance, don't install their own
> signal handlers at all, and rely on readline's signal handlers entirely.
>
> What do you suggest? Conditionalizing this behavior on whether or not
> rl_persistent_signal_handlers is set? Having readline set a flag if it
> prints the signal char info?

I did some experimentation at my end and updated my sample program to
make use of rl_persistent_signal_handlers.  I agree with your first
suggestion, having the rl_echo_signal_char call conditional on
rl_persistent_signal_handlers (when in CALLBACK mode) seems to allow for
both options.

Again, I'm happy to share my sample programs if that helps at all.

There's an updated patch below.

Thanks,
Andrew

---

commit c212adc4bb6357545ec3fe1326128dfc25821e6b
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Jun 1 11:21:04 2023 +0100

    readline: don't always call rl_echo_signal_char in callback mode
    
    This patch was a result of some GDB work that is discussed in this
    thread:
    
      
https://inbox.sourceware.org/gdb-patches/20230518061046.17837-1-tdevries@suse.de/
    
    In callback mode, and if rl_persistent_signal_handlers is off, then
    most signals will be delivered directly to the applications signal
    handler.  If the application wants to see the signal char then the
    application will take care of calling rl_echo_signal_char.
    
    However, readline currently always calls rl_echo_signal_char for
    signals seen within readline, this means that if a happens to arrive
    at the right/wrong moment then rl_echo_signal_char will always be
    called, whether the application calls this itself or not.
    
    In callback mode when rl_persistent_signal_handlers is on then the
    readline signal handler is always in place for the duration of the
    readline prompt being in use.  In this case it does make sense to
    always call rl_echo_signal_char.  By the time the application's signal
    handler is invoked we are on the way out of readline, and it is no
    longer the applications job to call rl_echo_signal_char.
    
    In non-callback mode, this patch changes nothing, rl_echo_signal_char
    is always called.

diff --git a/signals.c b/signals.c
index ec835e5..0df184d 100644
--- a/signals.c
+++ b/signals.c
@@ -267,7 +267,11 @@ _rl_handle_signal (int sig)
        sigprocmask (SIG_BLOCK, &set, &oset);
 #endif
 
-      rl_echo_signal_char (sig);
+#if defined (READLINE_CALLBACKS)
+      if (rl_persistent_signal_handlers)
+#endif
+       rl_echo_signal_char (sig);
+
       rl_cleanup_after_signal ();
 
       /* At this point, the application's signal handler, if any, is the