From b3cca3938332011ef9b2454ba5a30f15863930ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Bri=C3=A8re?= Date: Fri, 28 Jun 2019 23:38:58 -0400 Subject: Make Readline non-blocking: Use Readline's callback interface When Twinkle is running in CLI mode and is sent a "quit" command to its local socket, it will currently not respond immediately, but rather wait until the next line has been read from its standard input (issue #143). This is due to the blocking nature of readline(), which only returns once a complete line has been read. Switching to Readline's "alternate" callback interface is the first step in addressing this issue. (As a bonus, this also fixes a bug where the line pointer returned by readline() was not freed correctly.) --- src/userintf.cpp | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/userintf.cpp b/src/userintf.cpp index f75a140..772184d 100644 --- a/src/userintf.cpp +++ b/src/userintf.cpp @@ -86,22 +86,29 @@ char * tw_command_generator (const char *text, int state) return ((char *)NULL); } -char *tw_readline(const char *prompt) +// Ugly hack to allow invoking methods on our object from within a C-style +// callback function. This relies on the object being a singleton. +static t_userintf *cb_user_intf; +// Callback method (a.k.a. "line handler") that will be invoked by Readline +// once a complete line has been read. +static void tw_readline_cb(char *line) { - static char *line = NULL; - if (!line) { + // EOF + cout << endl; + // Calling this from the line handler prevents one extra + // prompt from being displayed. (The duplicate call later on + // will not be an issue.) + rl_callback_handler_remove(); + + cb_user_intf->cmd_quit(); + } else { + if (*line) { + add_history(line); + cb_user_intf->exec_command(line); + } free(line); - line = NULL; - } - - line = readline(prompt); - - if (line && *line) { - add_history(line); } - - return line; } ///////////////////////////// @@ -2206,16 +2213,15 @@ void t_userintf::run(void) { read_history(sys_config->get_history_file().c_str()); stifle_history(CLI_MAX_HISTORY_LENGTH); + // Additional stuff for using the Readline callback interface + cb_user_intf = this; + rl_callback_handler_install(CLI_PROMPT, tw_readline_cb); while (!end_interface) { - char *command_line = tw_readline(CLI_PROMPT); - if (!command_line){ - cout << endl; - break; - } - - exec_command(command_line); + rl_callback_read_char(); } + + rl_callback_handler_remove(); // Terminate phone functions write_history(sys_config->get_history_file().c_str()); -- cgit v1.2.3 From cedd61a753785831f71ae5582c134c816c92700d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Bri=C3=A8re?= Date: Sat, 29 Jun 2019 08:29:54 -0400 Subject: Make Readline non-blocking: Relay SIGWINCH to Readline When using Readline's callback interface, signals are (by default) only captured within the duration of the rl_callback_read_char() call. It is therefore expected of the application to capture SIGWINCH on its own, and notify Readline of the fact. (While it's possible to change this with rl_persistent_signal_handlers, some signals, such as SIGHUP, will still only be *processed* during that call, making it a somewhat unappealing solution. This all could be alleviated by calling rl_check_signals() periodically, but that function was only introduced in Readline 8.0, which is far too recent.) Note that we are using signal(2) and not sigaction(2), depite the various warnings that come with it, mostly because it's what is already present in the codebase. --- src/userintf.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'src') diff --git a/src/userintf.cpp b/src/userintf.cpp index 772184d..6534e99 100644 --- a/src/userintf.cpp +++ b/src/userintf.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include "address_book.h" #include "events.h" #include "line.h" @@ -111,6 +112,14 @@ static void tw_readline_cb(char *line) } } +// SIGWINCH handler to help us relay that information to Readline +static int sigwinch_received; +static void sigwinch_handler(int signum) +{ + sigwinch_received = 1; + signal(SIGWINCH, sigwinch_handler); +} + ///////////////////////////// // Private ///////////////////////////// @@ -2215,13 +2224,20 @@ void t_userintf::run(void) { // Additional stuff for using the Readline callback interface cb_user_intf = this; + signal(SIGWINCH, sigwinch_handler); rl_callback_handler_install(CLI_PROMPT, tw_readline_cb); while (!end_interface) { + // Relay any SIGWINCH to Readline + if (sigwinch_received) { + rl_resize_terminal(); + sigwinch_received = 0; + } rl_callback_read_char(); } rl_callback_handler_remove(); + signal(SIGWINCH, SIG_DFL); // Terminate phone functions write_history(sys_config->get_history_file().c_str()); -- cgit v1.2.3 From d26fa24087265dd4e9dccf0db261da0e6f093c8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Bri=C3=A8re?= Date: Fri, 28 Jun 2019 23:48:04 -0400 Subject: Make Readline non-blocking: Wrap select(2) around Readline call By only invoking rl_callback_read_char() when there is actual input on stdin, we can now avoid blocking on Readline calls. --- src/userintf.cpp | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/userintf.cpp b/src/userintf.cpp index 6534e99..21888ac 100644 --- a/src/userintf.cpp +++ b/src/userintf.cpp @@ -17,9 +17,13 @@ #include #include +#include #include #include #include +#include +#include +#include #include "address_book.h" #include "events.h" #include "line.h" @@ -2228,12 +2232,30 @@ void t_userintf::run(void) { rl_callback_handler_install(CLI_PROMPT, tw_readline_cb); while (!end_interface) { + fd_set fds; + FD_ZERO(&fds); + FD_SET(fileno(rl_instream), &fds); + + int ret = select(FD_SETSIZE, &fds, NULL, NULL, NULL); + if ((ret == -1) && (errno != EINTR)) { + string msg("select() failed: "); + msg += get_error_str(errno); + ui->cb_show_msg(msg, MSG_CRITICAL); + break; + } // Relay any SIGWINCH to Readline if (sigwinch_received) { rl_resize_terminal(); sigwinch_received = 0; } - rl_callback_read_char(); + if (ret == -1) { + // errno == EINTR + continue; + } + + if (FD_ISSET(fileno(rl_instream), &fds)) { + rl_callback_read_char(); + } } rl_callback_handler_remove(); -- cgit v1.2.3 From 18f2b023bc2c1ea84e7690b604c196580ac8f110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Bri=C3=A8re?= Date: Sat, 29 Jun 2019 08:31:08 -0400 Subject: Make Readline non-blocking: Use a self-pipe to break the Readline loop Now that we are no longer blocking on Readline calls, we can set up a self-pipe that will let us break out of the Readline loop upon receiving a "quit" command on our local socket, thus (finally) fixing issue #143. (Thanks to https://stackoverflow.com/a/27662212 for the tip!) Fixes #143 --- src/userintf.cpp | 31 +++++++++++++++++++++++++++++++ src/userintf.h | 1 + 2 files changed, 32 insertions(+) (limited to 'src') diff --git a/src/userintf.cpp b/src/userintf.cpp index 21888ac..5268a7c 100644 --- a/src/userintf.cpp +++ b/src/userintf.cpp @@ -18,12 +18,14 @@ #include #include #include +#include #include #include #include #include #include #include +#include #include "address_book.h" #include "events.h" #include "line.h" @@ -1483,6 +1485,10 @@ bool t_userintf::exec_quit(const list command_list) { void t_userintf::do_quit(void) { end_interface = true; + // Signal the main thread that it should interrupt Readline + if (break_readline_loop_pipe[1] != -1) { + write(break_readline_loop_pipe[1], "X", 1); + } } bool t_userintf::exec_help(const list command_list) { @@ -2220,6 +2226,27 @@ void t_userintf::run(void) { // Initialize phone functions phone->init(); + // Set up the self-pipe used to interrupt Readline + if (pipe(break_readline_loop_pipe) == 0) { + // Mark both file descriptors as close-on-exec for good measure + for (int i = 0; i < 2; i++) { + int flags = fcntl(break_readline_loop_pipe[i], F_GETFD); + if (flags != -1) { + flags |= FD_CLOEXEC; + fcntl(break_readline_loop_pipe[i], F_SETFD, flags); + } + } + } else { + // Not fatal -- we just won't be able to interrupt Readline + string msg("pipe() failed: "); + msg += get_error_str(errno); + ui->cb_show_msg(msg, MSG_WARNING); + + // Mark both file descriptors as invalid + break_readline_loop_pipe[0] = -1; + break_readline_loop_pipe[1] = -1; + } + //Initialize GNU readline functions rl_attempted_completion_function = tw_completion; using_history(); @@ -2232,9 +2259,13 @@ void t_userintf::run(void) { rl_callback_handler_install(CLI_PROMPT, tw_readline_cb); while (!end_interface) { + // File descriptors we are watching (stdin + self-pipe) fd_set fds; FD_ZERO(&fds); FD_SET(fileno(rl_instream), &fds); + if (break_readline_loop_pipe[0] != -1) { + FD_SET(break_readline_loop_pipe[0], &fds); + } int ret = select(FD_SETSIZE, &fds, NULL, NULL, NULL); if ((ret == -1) && (errno != EINTR)) { diff --git a/src/userintf.h b/src/userintf.h index 48a4c50..2c6d96d 100644 --- a/src/userintf.h +++ b/src/userintf.h @@ -62,6 +62,7 @@ protected: private: bool end_interface; // indicates if interface loop should quit + int break_readline_loop_pipe[2]; // pipe used to interrupt Readline list all_commands; // list of all commands t_tone_gen *tone_gen; // tone generator for ringing -- cgit v1.2.3