How to make libreadline or libedit cope with SIGALRM?

160 views Asked by At

I am writing a Linux program that interacts with some custom hardware via GPIO lines. It uses a timer periodically delivering SIGALRM in order to manage the timing of these interactions, and there is quite some work performed within the signal handler. The program offers a command-line interface, and I would love to have the line-editing convenience provided by libreadline. Unfortunately, it would seem libreadline does not cope well with those periodic signals. I also tried the readline compatibility mode of libedit (a.k.a. “editline”), with no luck.

Is there an easy way to have readline capability in a program that receives numerous SIGALRM signals?

Symptoms

When using libreadline, if the timer is running, some of the characters typed are randomly echoed twice. For example, typing "help" may result in "heelp" being displayed on the terminal. The issue is only apparent in the echo: the program does receive the word as typed (i.e. "help").

When using libedit in readline compatibility mode, if the timer is running, readline() returns NULL whenever it is interrupted by the SIGALRM signal.

When the timer is stopped, everything works as expected, both with libreadline and with libedit.

Environment

Ubuntu 20.04 with the latest apt packages libreadline-dev (version 8.0-4) and libedit-dev (3.1-20191231-1). The program will eventually be deployed on a Raspberry Pi running Raspberry Pi OS.

Example code

Here is an attempt at a minimal(ish), reproducible example:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <signal.h>
#include <sys/time.h>
#ifdef USE_LIBEDIT
# include <editline/readline.h>
#else
# include <readline/readline.h>
#endif

#define PERIOD_US  1000  // 1.0 ms
#define DELAY_NS 700000  // 0.7 ms

/* Timer settings. */
const struct itimerval interval_off = {
    .it_value    = { .tv_sec = 0, .tv_usec = 0 },
    .it_interval = { .tv_sec = 0, .tv_usec = 0 }
};
const struct itimerval interval_1ms = {
    .it_value    = { .tv_sec = 0, .tv_usec = PERIOD_US },
    .it_interval = { .tv_sec = 0, .tv_usec = PERIOD_US }
};

static void sigalrm_callback(int signum)
{
    (void) signum;

    // Simulate work by busy-wating for DELAY_NS.
    struct timespec end, now;
    clock_gettime(CLOCK_MONOTONIC, &end);
    end.tv_nsec += DELAY_NS;
    end.tv_sec  += end.tv_nsec / 1000000000;
    end.tv_nsec %= 1000000000;
    do
        clock_gettime(CLOCK_MONOTONIC, &now);
    while (now.tv_sec < end.tv_sec
            || (now.tv_sec == end.tv_sec && now.tv_nsec < end.tv_nsec));
}

static int should_quit = 0;

/* Interpret and free line. */
void interpret(char *line)
{
    if (!line) {
        printf("Got NULL line\n");
    } else if (line[0] == '\0') {
        /* Ignore empty line. */
    } else if (strcmp(line, "help") == 0) {
        puts("help    print this help\n"
             "start   start the interval timer at 1 kHz\n"
             "stop    stop the interval timer\n"
             "quit    end the program");
    } else if (strcmp(line, "start") == 0) {
        setitimer(ITIMER_REAL, &interval_1ms, NULL);
        printf("Periodic timer started.\n");
    } else if (strcmp(line, "stop") == 0) {
        setitimer(ITIMER_REAL, &interval_off, NULL);
        printf("Periodic timer stopped.\n");
    } else if (strcmp(line, "quit") == 0) {
        should_quit = 1;
    } else {
        printf("Unknown command \"%s\".\n", line);
    }
    free(line);
}

int main(void)
{
    /* Catch SIGALRM. */
    struct sigaction action;
    action.sa_handler = sigalrm_callback;
    sigemptyset(&action.sa_mask);
    action.sa_flags = 0;
    sigaction(SIGALRM, &action, NULL);

    /* Process commands. */
    while (!should_quit) {
        char *line = readline("> ");
        interpret(line);
    }

    return EXIT_SUCCESS;
}

Compile either with

gcc -O2 readline-alrm.c -lreadline -o readline-alrm

or

gcc -O2 -DUSE_LIBEDIT readline-alrm.c -ledit -o readline-alrm

Edit: I moved the command interpreter out of main().

1

There are 1 answers

0
Edgar Bonet On BEST ANSWER

I found a solution. The issue can be fixed by using the asynchronous “alternate” interface of readline. In the example program from the question, replace main() with this:

#include <unistd.h>
#include <errno.h>
#include <sys/select.h>

int main(void)
{
    /* Catch SIGALRM. */
    struct sigaction action;
    action.sa_handler = sigalrm_callback;
    sigemptyset(&action.sa_mask);
    action.sa_flags = 0;
    sigaction(SIGALRM, &action, NULL);

    /* Process commands. */
    rl_callback_handler_install("> ", interpret);
    while (!should_quit) {
        fd_set fds;
        FD_ZERO(&fds);
        FD_SET(STDIN_FILENO, &fds);
        int ret = select(STDIN_FILENO + 1, &fds, NULL, NULL, NULL);
#ifdef FIX_BUG
        /* Restart on interrupted system call. */
        if (ret == -1 && errno == EINTR)
                continue;
#endif
        if (FD_ISSET(STDIN_FILENO, &fds))
            rl_callback_read_char();
    }
    putchar('\n');
    rl_callback_handler_remove();

    return EXIT_SUCCESS;
}

Note that the issue is fixed only if the macro FIX_BUG is defined.

Here is my understanding of the problem. When select() is interrupted by a signal, it returns -1 with errno set to EINTR. When this happens, most of the time the file descriptor set fds turns out empty. In some cases, however, select() leaves STDIN_FILENO within fds. In this case, the program should not attempt to process its input. If we call rl_callback_read_char() despite select() having returned -1, then we have the double-echo issue.

I wonder whether this is a bug in the implementation of readline(). Maybe it is implemented on top of this async interface and it fails to check the value returned by select()?