fix(editor): prevent UI redraw conflicts during editor suspend/resume
All checks were successful
CI Code / Check spelling (push) Successful in 16s
CI Code / Check coding style (push) Successful in 31s
CI Code / Code Coverage (push) Successful in 3m36s
CI Code / Linux (debian) (push) Successful in 4m43s
CI Code / Linux (ubuntu) (push) Successful in 4m54s
CI Code / Linux (arch) (push) Successful in 6m46s

Guard against re-entrant editor launches, abort editor on Ctrl-Z,
and centralize suspend-aware redraws through prof_doupdate() to prevent
terminal corruption when SIGTSTP is received while editor is active.

Removes SIGUSR1 editor escape in favor of standard pkill <editor> recovery.
This commit is contained in:
2026-06-01 12:21:34 +00:00
parent 72f4f186da
commit c1093db090
11 changed files with 187 additions and 21 deletions

View File

@@ -2638,7 +2638,8 @@ static const struct cmd_t command_defs[] = {
CMD_DESC(
"Spawn external editor to edit message. "
"After editing the inputline may appear empty. Press enter to send the text anyways. "
"Use /executable to set your favourite editor." )
"Use /executable to set your favourite editor. "
"Ctrl-Z in the editor aborts the session and returns to Profanity." )
},
{ CMD_PREAMBLE("/correct-editor",

View File

@@ -341,7 +341,7 @@ sv_ev_room_message(ProfMessage* message)
status_bar_active(num, WIN_MUC, mucwin->roomjid);
if ((g_strcmp0(mynick, message->from_jid->resourcepart) != 0) && (prefs_get_boolean(PREF_BEEP))) {
beep();
ui_beep();
}
// not currently on groupchat window
@@ -349,7 +349,7 @@ sv_ev_room_message(ProfMessage* message)
status_bar_new(num, WIN_MUC, mucwin->roomjid);
if ((g_strcmp0(mynick, message->from_jid->resourcepart) != 0) && (prefs_get_boolean(PREF_FLASH))) {
flash();
ui_flash();
}
cons_show_incoming_room_message(message->from_jid->resourcepart, mucwin->roomjid, num, mention, triggers, mucwin->unread, window);

View File

@@ -70,6 +70,38 @@ static void _connect_default(const char* const account);
pthread_mutex_t lock;
static gboolean force_quit = FALSE;
static volatile sig_atomic_t suspend_requested = FALSE;
static volatile sig_atomic_t cont_requested = FALSE;
static void
_handle_suspend_signals(void)
{
if (cont_requested) {
cont_requested = FALSE;
if (!editor_is_active()) {
clearok(stdscr, TRUE);
ui_resize();
}
}
if (editor_check_stopped()) {
log_info("[Editor] stopped; aborting back to profanity");
suspend_requested = FALSE;
editor_emergency_kill();
} else if (suspend_requested) {
suspend_requested = FALSE;
if (editor_is_active()) {
log_info("[Editor] SIGTSTP from vim group; deferring to next poll");
} else {
log_info("[Suspend] dropping to shell");
if (!isendwin()) {
endwin();
}
kill(0, SIGSTOP);
}
}
}
void
prof_run(gchar* log_level, gchar* account_name, gchar* config_file, gchar* log_file, gchar* theme_name, gchar** commands)
{
@@ -93,6 +125,7 @@ prof_run(gchar* log_level, gchar* account_name, gchar* config_file, gchar* log_f
int waittime = 0;
while (cont && !force_quit) {
log_stderr_handler();
_handle_suspend_signals();
session_check_autoaway();
line = commands ? *commands : inp_readline();
@@ -182,6 +215,18 @@ sigterm_handler(int sig)
force_quit = TRUE;
}
static void
sigtstp_handler(int sig)
{
suspend_requested = TRUE;
}
static void
sigcont_handler(int sig)
{
cont_requested = TRUE;
}
static void
_init(char* log_level, char* config_file, char* log_file, char* theme_name)
{
@@ -189,7 +234,8 @@ _init(char* log_level, char* config_file, char* log_file, char* theme_name)
// ignore SIGPIPE
signal(SIGPIPE, SIG_IGN);
signal(SIGINT, SIG_IGN);
signal(SIGTSTP, SIG_IGN);
signal(SIGTSTP, sigtstp_handler);
signal(SIGCONT, sigcont_handler);
signal(SIGWINCH, ui_sigwinch_handler);
signal(SIGTERM, sigterm_handler);
signal(SIGHUP, sigterm_handler);

View File

@@ -32,9 +32,15 @@ typedef struct EditorContext
void* user_data;
} EditorContext;
static gboolean editor_active = FALSE; // re-entrancy guard
static pid_t editor_pid = 0;
static void
_editor_exit_cb(GPid pid, gint status, gpointer data)
{
editor_active = FALSE;
editor_pid = 0;
EditorContext* ctx = data;
gchar* contents = NULL;
GError* error = NULL;
@@ -52,8 +58,10 @@ _editor_exit_cb(GPid pid, gint status, gpointer data)
cons_show_error("Could not read edited content: %s", error->message);
g_error_free(error);
}
} else {
cons_show_error("Editor exited with error status %d", WEXITSTATUS(status));
} else if (WIFEXITED(status)) {
cons_show_error("Editor exited with status %d", WEXITSTATUS(status));
} else if (WIFSIGNALED(status)) {
cons_show("Editor session cancelled.");
}
if (remove(ctx->filename) != 0) {
@@ -70,6 +78,11 @@ _editor_exit_cb(GPid pid, gint status, gpointer data)
gboolean
launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* data), void* user_data)
{
if (editor_active) {
cons_show_error("An editor session is already active.");
return TRUE;
}
auto_gchar gchar* filename = NULL;
auto_gerror GError* glib_error = NULL;
const char* jid = connection_get_barejid();
@@ -118,6 +131,7 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
ctx->callback = callback;
ctx->user_data = user_data;
editor_active = TRUE;
ui_suspend();
// Force GLib to install its SIGCHLD handler before forking. Creating a
@@ -129,6 +143,7 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
pid_t pid = fork();
if (pid == -1) {
log_error("[Editor] Failed to fork: %s", strerror(errno));
editor_active = FALSE;
ui_resume();
ui_resize();
cons_show_error("Failed to start editor: %s", strerror(errno));
@@ -139,7 +154,8 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
} else if (pid == 0) {
// Child process: Inherits TTY from parent
// Reset signal handlers that profanity sets so the editor doesn't inherit them
// SIGTSTP=SIG_DFL lets vim's :stop / Ctrl-Z work; profanity catches
// the STOPPED state via editor_check_stopped() and drops to the shell.
signal(SIGINT, SIG_DFL);
signal(SIGTSTP, SIG_DFL);
signal(SIGPIPE, SIG_DFL);
@@ -151,7 +167,39 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
}
// Parent process: Watch the child asynchronously
editor_pid = pid;
g_child_watch_add((GPid)pid, _editor_exit_cb, ctx);
g_strfreev(editor_argv);
return FALSE;
}
void
editor_emergency_kill(void)
{
if (!editor_active || editor_pid <= 0) {
return;
}
log_warning("[Editor] aborting editor pid %d", editor_pid);
kill(editor_pid, SIGCONT); // wake if STOPPED; no-op otherwise
kill(editor_pid, SIGTERM);
// _editor_exit_cb will restore the UI when the child reaps.
}
gboolean
editor_is_active(void)
{
return editor_active;
}
gboolean
editor_check_stopped(void)
{
// Poll for STOPPED without reaping (g_child_watch handles exit via its
// own WNOHANG). Lets prof_run drop the group to the shell on Ctrl-Z.
if (!editor_active || editor_pid <= 0) {
return FALSE;
}
int status;
pid_t r = waitpid(editor_pid, &status, WNOHANG | WUNTRACED);
return (r == editor_pid && WIFSTOPPED(status));
}

View File

@@ -16,4 +16,13 @@
gboolean launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* data), void* user_data);
void editor_emergency_kill(void); // SIGCONT+SIGTERM the editor child
// TRUE if editor child is STOPPED (Ctrl-Z / :stop / gdb-attach). Does not
// reap; g_child_watch keeps handling exit.
gboolean editor_check_stopped(void);
// Editor-session flag accessor. Used by prof_run on SIGCONT resume.
gboolean editor_is_active(void);
#endif

View File

@@ -327,7 +327,7 @@ chatwin_incoming_msg(ProfChatWin* chatwin, ProfMessage* message, gboolean win_cr
cons_show_incoming_message(display_name, num, chatwin->unread, window);
if (prefs_get_boolean(PREF_FLASH)) {
flash();
ui_flash();
}
chatwin->unread++;
@@ -364,7 +364,7 @@ chatwin_incoming_msg(ProfChatWin* chatwin, ProfMessage* message, gboolean win_cr
wins_add_quotes_ac(window, message->plain, FALSE);
if (prefs_get_boolean(PREF_BEEP)) {
beep();
ui_beep();
}
}

View File

@@ -54,6 +54,7 @@
static int inp_size;
static gboolean perform_resize = FALSE;
static gboolean ui_suspended = FALSE;
static GTimer* ui_idle_time;
static WINDOW* main_scr;
@@ -63,6 +64,14 @@ static Display* display;
static void _ui_draw_term_title(void);
// Fail-safe gate: authoritative flag (doesn't depend on isendwin staying
// TRUE), plus the regular endwin check for shutdown.
static gboolean
_ui_redraw_suspended(void)
{
return ui_suspended || isendwin();
}
static void
_ui_close(void)
{
@@ -120,8 +129,7 @@ ui_sigwinch_handler(int sig)
void
ui_update(void)
{
// UI is suspended
if (isendwin()) {
if (_ui_redraw_suspended()) {
return;
}
@@ -138,7 +146,7 @@ ui_update(void)
title_bar_update_virtual();
status_bar_draw();
inp_put_back();
doupdate();
prof_doupdate();
if (perform_resize) {
perform_resize = FALSE;
@@ -180,21 +188,49 @@ void
ui_suspend(void)
{
inp_suspend();
// Flush before flipping the flag (prof_doupdate skips while suspended).
prof_doupdate();
ui_suspended = TRUE;
endwin();
}
void
ui_resume(void)
{
ui_suspended = FALSE;
clearok(stdscr, TRUE); // ncurses can't see vim's screen clobber
refresh();
inp_resume();
}
void
ui_beep(void)
{
if (!isendwin()) {
beep();
}
}
void
ui_flash(void)
{
if (!isendwin()) {
flash();
}
}
void
prof_doupdate(void)
{
if (!_ui_redraw_suspended()) {
doupdate();
}
}
void
ui_resize(void)
{
// UI is suspended
if (isendwin()) {
if (_ui_redraw_suspended()) {
return;
}
@@ -216,8 +252,7 @@ ui_resize(void)
void
ui_redraw(void)
{
// UI is suspended
if (isendwin()) {
if (_ui_redraw_suspended()) {
return;
}

View File

@@ -182,7 +182,11 @@ inp_readline(void)
{
// UI is suspended
if (is_suspended || isendwin()) {
// Mirror the normal-path unlock so workers aren't starved
// for the whole editor session.
pthread_mutex_unlock(&lock);
g_usleep(100000); // 100ms
pthread_mutex_lock(&lock);
return NULL;
}
@@ -311,11 +315,13 @@ inp_get_line(void)
werase(inp_win);
wmove(inp_win, 0, 0);
_inp_win_update_virtual();
doupdate();
prof_doupdate();
char* line = NULL;
while (!line) {
line = inp_readline();
ui_update();
while (g_main_context_iteration(NULL, FALSE)) // non-blocking; bounded by ready sources
;
}
status_bar_clear_prompt();
return line;
@@ -338,12 +344,14 @@ inp_get_password(void)
werase(inp_win);
wmove(inp_win, 0, 0);
_inp_win_update_virtual();
doupdate();
prof_doupdate();
char* password = NULL;
get_password = TRUE;
while (!password) {
password = inp_readline();
ui_update();
while (g_main_context_iteration(NULL, FALSE)) // non-blocking; bounded by ready sources
;
}
get_password = FALSE;
status_bar_clear_prompt();
@@ -459,7 +467,7 @@ _inp_write(char* line, int offset)
_inp_win_handle_scroll();
_inp_win_update_virtual();
doupdate();
prof_doupdate();
}
static int

View File

@@ -57,7 +57,7 @@ privwin_incoming_msg(ProfPrivateWin* privatewin, ProfMessage* message)
privatewin->unread++;
if (prefs_get_boolean(PREF_FLASH)) {
flash();
ui_flash();
}
}
@@ -65,7 +65,7 @@ privwin_incoming_msg(ProfPrivateWin* privatewin, ProfMessage* message)
wins_add_quotes_ac(window, message->plain, TRUE);
if (prefs_get_boolean(PREF_BEEP)) {
beep();
ui_beep();
}
if (notify) {

View File

@@ -43,6 +43,9 @@ void ui_load_colours(void);
void ui_update(void);
void ui_redraw(void);
void ui_resize(void);
// doupdate() wrapper that skips when the UI is suspended. Use this instead
// of raw doupdate() to avoid the per-site whitelist class of bug.
void prof_doupdate(void);
void ui_focus_win(ProfWin* window);
void ui_sigwinch_handler(int sig);
void ui_handle_otr_error(const char* const barejid, const char* const message);
@@ -84,6 +87,8 @@ void ui_handle_recipient_error(const char* const recipient, const char* const er
void ui_handle_error(const char* const err_msg);
void ui_clear_win_title(void);
void ui_goodbye_title(void);
void ui_beep(void);
void ui_flash(void);
void ui_handle_room_configuration_form_error(const char* const roomjid, const char* const message);
void ui_handle_room_config_submit_result(const char* const roomjid);
void ui_handle_room_config_submit_result_error(const char* const roomjid, const char* const message);

View File

@@ -85,6 +85,10 @@ void
ui_resize(void)
{
}
void
prof_doupdate(void)
{
}
void
ui_focus_win(ProfWin* win)
@@ -1475,3 +1479,13 @@ ui_is_suspended(void)
{
return FALSE;
}
void
ui_beep(void)
{
}
void
ui_flash(void)
{
}