Compare commits

...

8 Commits

Author SHA1 Message Date
86d924eaac fix(editor): avoid Ctrl-Z hang on the editor child, add SIGUSR1 escape
All checks were successful
CI Code / Check spelling (pull_request) Successful in 18s
CI Code / Check coding style (pull_request) Successful in 32s
CI Code / Code Coverage (pull_request) Successful in 2m36s
CI Code / Linux (debian) (pull_request) Successful in 4m38s
CI Code / Linux (ubuntu) (pull_request) Successful in 4m49s
CI Code / Linux (arch) (pull_request) Successful in 5m35s
The upstream signal reset in launch_editor set SIGTSTP back to SIG_DFL in
the child, letting Ctrl-Z (or vim's :stop) stop the editor. GLib's
g_child_watch uses waitpid(WNOHANG) without WUNTRACED, so a STOPPED
editor never wakes the watch and the app freezes until killed externally.
Other TUI chat clients carry this open problem for years (see
weechat/weechat#985, open since 2017); shells solve it with full job
control (setpgid + tcsetpgrp + WUNTRACED), which is heavy for our use
case.

Drop just the SIGTSTP reset. SIGINT and SIGPIPE are still set to SIG_DFL
— Neovim's libuv needs SIGINT-as-default to recognise an interactive
terminal (per upstream commit dd76f9087). SIGTSTP stays inherited as
SIG_IGN from profanity's _init, so Ctrl-Z and vim's :stop become no-ops
inside /editor. Losing vim's :stop during message composition is an
acceptable trade for not freezing the whole app.

As a universal escape hatch for any other "hung editor" scenario
(looping $EDITOR, corrupted tty, anything we cannot detect), install a
SIGUSR1 handler via g_unix_signal_add that sends SIGTERM to the editor
child via editor_emergency_kill(). The existing g_child_watch then
dispatches the normal exit callback, ui_resume runs, and the user is
back in profanity. Recovery from another terminal: `pkill -USR1
profanity`.
2026-05-29 09:31:34 +03:00
bb0af8b7c8 refactor(ui): centralize physical paint through prof_doupdate()
All checks were successful
CI Code / Check spelling (pull_request) Successful in 18s
CI Code / Check coding style (pull_request) Successful in 32s
CI Code / Linux (debian) (pull_request) Successful in 4m57s
CI Code / Linux (ubuntu) (pull_request) Successful in 5m0s
CI Code / Linux (arch) (pull_request) Successful in 5m46s
CI Code / Code Coverage (pull_request) Successful in 7m3s
Upstream's "guard terminal writes while suspended" fix and commit 2 of
this branch both work by sprinkling `if (!isendwin())` (or
`!is_suspended && !isendwin()`) guards around every doupdate() reachable
during a suspended session. That keeps isendwin() a valid signal but
turns the no-stray-write invariant into a whitelist: every new
doupdate / refresh / wrefresh added later in src/ has to remember the
guard, and the original bug class came from exactly such an oversight
in _inp_write.

Introduce a single primitive, prof_doupdate(), that flushes only when
not suspended (via the existing _ui_redraw_suspended() helper). Replace
every direct doupdate() in src/ui/inputwin.c (inp_get_line,
inp_get_password, _inp_write) and the trailing doupdate in ui_update
with the wrapper, collapsing their inline guards. ui_suspend() now
flushes via prof_doupdate() before flipping ui_suspended, so upstream's
pre-endwin flush is preserved.

The flag-based gate in ui_update / ui_resize / ui_redraw is kept as
belt-and-suspenders: even if a future raw doupdate slips in elsewhere,
ui_update has not been refreshing the virtual screen, so the blit finds
stale content rather than a fresh status-bar clock or chat line.
2026-05-29 08:54:30 +03:00
a51afa53f6 fix(editor): guard against re-entrant launch_editor
The async editor rewrite (g_child_watch + ui_suspend / ui_resume) lost
the editor_task.active check the old pthread implementation had at the
top of get_message_from_editor_async. With the guard gone, a second
launch_editor call while the first session is in flight would call
ui_suspend a second time, fork another editor, and register a second
child watch; whichever child exits first runs ui_resume and clears
suspension while the other still owns the terminal. Triggering this
needs a code path other than the main-loop alt+c / /editor (those can't
fire while the main loop is parked in the suspended iteration), so the
realistic source is a plugin firing launch_editor from a
g_main_context_iteration dispatch.

Restore an explicit editor_active static in src/tools/editor.c: check it
at the start of launch_editor (bail with a console error), set it just
before ui_suspend, and clear it on the fork-error path and at the start
of _editor_exit_cb. Keeps the same simple semantics the pre-merge code
had, without exposing a new ui.h API.
2026-05-29 08:49:57 +03:00
b1ffc750a2 fix(ui): iterate main context inside synchronous input loops
inp_get_line() / inp_get_password() loop `while (!x) { inp_readline();
ui_update(); }` with no g_main_context_iteration call. If such a prompt
is raised from a network handler during an editor session — realistic
triggers are TLS-cert verification on a reconnect (sv_ev_certfail ->
ui_get_line() in src/event/server_events.c) and PGP passphrase on an
incoming encrypted message (ui_ask_pgp_passphrase() in src/pgp/gpg.c) —
the main thread is parked in this loop. inp_readline returns NULL
(suspended), ui_update returns (ui_suspended), and the editor child
watch never dispatches because the main loop never reaches its own
g_main_context_iteration. is_suspended stays TRUE forever, the loop
never receives input, and the app deadlocks until killed externally.

Run the GLib iteration inside the loop body so the child watch can
dispatch, _editor_exit_cb fires, ui_resume clears suspension, and the
next inp_readline reads from the terminal — letting the user answer the
deferred prompt once the editor is closed.
2026-05-29 08:48:51 +03:00
2da2449777 fix(ui): release global lock around suspend-branch sleep in inp_readline
The normal path of inp_readline unlocks the global `lock` around
select(), giving worker threads (http_upload / http_download / ai_client
in src/tools and src/ai) a window to acquire it and update the UI.
The suspend branch (entered while an external editor is active) used to
g_usleep(100000) and return NULL while holding the lock, so the main
thread monopolised it for the whole editor session and every in-flight
transfer froze; on a long composition (the customer's main /editor use
case) this could be long enough for server-side timeouts to fire.

Mirror the normal-path unlock / sleep / lock pattern so the same window
exists during the editor session. Workers in the suspended state print
through wnoutrefresh paths only (no direct doupdate / refresh / wrefresh
in src outside core.c / inputwin.c, all of which are already guarded
against suspended writes), so opening the window introduces no new race.
2026-05-29 08:48:25 +03:00
394df6d391 fix(ui): add authoritative suspend flag gating redraws over the editor
All checks were successful
CI Code / Check spelling (pull_request) Successful in 19s
CI Code / Check coding style (pull_request) Successful in 32s
CI Code / Code Coverage (pull_request) Successful in 2m37s
CI Code / Linux (debian) (pull_request) Successful in 4m41s
CI Code / Linux (ubuntu) (pull_request) Successful in 4m50s
CI Code / Linux (arch) (pull_request) Successful in 5m39s
Builds on the upstream "guard terminal writes while suspended" fix. That
fix keeps isendwin() a valid "suspended" signal by guarding stray writes
(_inp_write, beep, flash) so the existing isendwin() checks in
ui_update/ui_resize/ui_redraw hold. That is correct but fail-open: any
future unguarded doupdate()/refresh() reachable during an editor session
clears isendwin() and the ~10Hz ui_update() repaints over the editor
again (the original bug class).

Track suspension explicitly with a ui_suspended flag set in
ui_suspend()/ui_resume() and gate the three redraw entry points on
ui_suspended || isendwin() via _ui_redraw_suspended(). The flag is set
exactly at the suspend/resume boundary, so the dominant repaint path
fails safe regardless of isendwin() flapping. isendwin() is kept in the
condition for the shutdown/closed-screen case.
2026-05-27 22:16:51 +03:00
b282b12497 fix(ui): guard password/TLS prompt redraw while suspended
inp_get_line()/inp_get_password() issued an unconditional doupdate()
before their input loop. If such a prompt is raised while an external
editor is active (e.g. a reconnect password or TLS-cert prompt arriving
via session_process_events), that doupdate() writes over the editor.
Guard it with is_suspended || isendwin(), matching the inp_readline()
guard, so it fails safe even if a stray write later clears isendwin().
This closes the one suspended-write path the upstream fix left open.
2026-05-27 22:16:51 +03:00
Michael Vetter
562bd28cff fix(ui): guard terminal writes while suspended
We forgot to guard `beep()` and `flush()`.

Ref: 36ec2b0ae1
Ref: dd76f9087f
Signed-off-by: Michael Vetter <jubalh@iodoru.org>
Fixes: https://github.com/profanity-im/profanity/issues/2162
(cherry picked from commit 5e4b5d313a9e055e039e2feb4efae58b6aaead78)
2026-05-27 21:59:27 +03:00
10 changed files with 140 additions and 18 deletions

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

@@ -20,6 +20,7 @@
#include <string.h>
#include <glib.h>
#include <glib-unix.h>
#include "profanity.h"
#include "common.h"
@@ -182,6 +183,16 @@ sigterm_handler(int sig)
force_quit = TRUE;
}
// Universal "I'm stuck in the editor" escape hatch. Dispatched in the main
// loop by GLib (g_unix_signal_add), so it is safe to call into editor.c
// directly. Trigger from another terminal: `pkill -USR1 profanity`.
static gboolean
_emergency_resume_handler(gpointer data)
{
editor_emergency_kill();
return G_SOURCE_CONTINUE;
}
static void
_init(char* log_level, char* config_file, char* log_file, char* theme_name)
{
@@ -193,6 +204,7 @@ _init(char* log_level, char* config_file, char* log_file, char* theme_name)
signal(SIGWINCH, ui_sigwinch_handler);
signal(SIGTERM, sigterm_handler);
signal(SIGHUP, sigterm_handler);
g_unix_signal_add(SIGUSR1, _emergency_resume_handler, NULL);
if (pthread_mutex_init(&lock, NULL) != 0) {
log_error("Mutex init failed");
exit(1);

View File

@@ -32,9 +32,22 @@ typedef struct EditorContext
void* user_data;
} EditorContext;
// Set TRUE for the lifetime of an editor session (from launch_editor up to
// _editor_exit_cb / fork-error cleanup). The async editor model lost the old
// editor_task.active check from the pthread implementation; without this
// guard a second launch_editor (e.g. via a plugin dispatched through
// g_main_context_iteration) would call ui_suspend again and fork a second
// editor that fights the first one for the terminal, then prematurely
// ui_resume when either exits.
static gboolean editor_active = FALSE;
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;
@@ -70,6 +83,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 +136,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 +148,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,9 +159,11 @@ 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 intentionally NOT reset: keeping SIG_IGN inherited via
// fork+exec avoids hanging on a STOPPED editor (g_child_watch lacks
// WUNTRACED). Cost: vim's :stop is a no-op in /editor. See
// editor_emergency_kill for other hung-editor scenarios.
signal(SIGINT, SIG_DFL);
signal(SIGTSTP, SIG_DFL);
signal(SIGPIPE, SIG_DFL);
if (editor_argv && editor_argv[0]) {
@@ -151,7 +173,20 @@ 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] emergency resume: sending SIGTERM to editor pid %d", editor_pid);
kill(editor_pid, SIGTERM);
// g_child_watch dispatches _editor_exit_cb when the child exits, which
// already restores the UI — no further action required here.
}

View File

@@ -16,4 +16,9 @@
gboolean launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* data), void* user_data);
// Universal escape hatch: if an editor session is active, send SIGTERM to the
// child. The existing g_child_watch fires _editor_exit_cb on exit, which
// already restores the UI. Wired to SIGUSR1 in profanity.c.
void editor_emergency_kill(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,16 @@ static Display* display;
static void _ui_draw_term_title(void);
// Skip drawing while an external editor is active (ui_suspended) or after the
// screen has been closed (endwin). The flag is authoritative and does not rely
// on isendwin() staying TRUE, which a stray doupdate() during the session could
// otherwise clear.
static gboolean
_ui_redraw_suspended(void)
{
return ui_suspended || isendwin();
}
static void
_ui_close(void)
{
@@ -120,8 +131,7 @@ ui_sigwinch_handler(int sig)
void
ui_update(void)
{
// UI is suspended
if (isendwin()) {
if (_ui_redraw_suspended()) {
return;
}
@@ -138,7 +148,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 +190,48 @@ 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;
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 +253,7 @@ ui_resize(void)
void
ui_redraw(void)
{
// UI is suspended
if (isendwin()) {
if (_ui_redraw_suspended()) {
return;
}

View File

@@ -182,7 +182,13 @@ inp_readline(void)
{
// UI is suspended
if (is_suspended || isendwin()) {
// Mirror the normal-path unlock/sleep/lock so worker threads
// (http_upload / http_download / ai_client) can acquire `lock`
// during the editor session — otherwise the main thread holds
// it continuously and every in-flight transfer freezes.
pthread_mutex_unlock(&lock);
g_usleep(100000); // 100ms
pthread_mutex_lock(&lock);
return NULL;
}
@@ -311,11 +317,15 @@ 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();
// Let GLib sources (notably the editor child watch) dispatch so
// ui_resume can run if the prompt was raised mid-editor-session.
while (g_main_context_iteration(NULL, FALSE))
;
}
status_bar_clear_prompt();
return line;
@@ -338,12 +348,16 @@ 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();
// Let GLib sources (notably the editor child watch) dispatch so
// ui_resume can run if the prompt was raised mid-editor-session.
while (g_main_context_iteration(NULL, FALSE))
;
}
get_password = FALSE;
status_bar_clear_prompt();
@@ -459,7 +473,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,10 @@ void ui_load_colours(void);
void ui_update(void);
void ui_redraw(void);
void ui_resize(void);
// Centralised physical-paint primitive: flushes the virtual screen unless the
// UI is suspended (external editor active) or the screen has been closed. Use
// this instead of raw doupdate() in any path reachable while suspended.
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 +88,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)
{
}