Compare commits

...

10 Commits

Author SHA1 Message Date
b90dec5be3 remove SIGUSR1 escape; pkill <editor> is the standard recovery
All checks were successful
CI Code / Check coding style (pull_request) Successful in 32s
CI Code / Check spelling (pull_request) Successful in 19s
CI Code / Linux (debian) (pull_request) Successful in 4m30s
CI Code / Code Coverage (pull_request) Successful in 3m15s
CI Code / Linux (ubuntu) (pull_request) Successful in 4m36s
CI Code / Linux (arch) (pull_request) Successful in 6m21s
The SIGUSR1 -> editor_emergency_kill wiring was a contrived escape
hatch. A console user with a stuck editor reaches for kill / pkill /
SIGKILL on the editor, not undocumented signals on the chat client.
editor_emergency_kill() stays for the internal Ctrl-Z abort path.
2026-05-31 15:16:32 +03:00
3f91cbc8b0 fix(ui): clearok(stdscr) in ui_resume to clear vim screen residue
All checks were successful
CI Code / Check spelling (pull_request) Successful in 16s
CI Code / Check coding style (pull_request) Successful in 30s
CI Code / Code Coverage (pull_request) Successful in 3m22s
CI Code / Linux (debian) (pull_request) Successful in 4m47s
CI Code / Linux (ubuntu) (pull_request) Successful in 4m59s
CI Code / Linux (arch) (pull_request) Successful in 6m58s
ncurses didn't know the editor clobbered the physical screen, so the
first refresh after vim exit was a no-op diff. Stale frame stayed
visible until the next keystroke triggered readline redisplay.
2026-05-31 14:54:18 +03:00
e4422ac7a1 fix(editor): Ctrl-Z aborts editor and returns to profanity
Some checks failed
CI Code / Check spelling (pull_request) Successful in 16s
CI Code / Check coding style (pull_request) Failing after 34s
CI Code / Code Coverage (pull_request) Successful in 3m1s
CI Code / Linux (debian) (pull_request) Successful in 4m43s
CI Code / Linux (ubuntu) (pull_request) Successful in 4m55s
CI Code / Linux (arch) (pull_request) Successful in 6m55s
Replaces the SIGTSTP=SIG_IGN inheritance approach (was commit 8) with a
detect-and-abort design that matches user expectation: Ctrl-Z inside
the editor brings you back to profanity instead of leaving the session
suspended.

- src/tools/editor.c: SIGTSTP reset back to SIG_DFL in the child so
  vim's :stop / Ctrl-Z actually stop the editor (revert previous
  SIG_IGN inheritance).
- src/tools/editor.c: new editor_check_stopped() — non-reaping waitpid
  poll for WIFSTOPPED. Lets prof_run detect Ctrl-Z / :stop / gdb-attach
  on the editor child.
- src/tools/editor.c: editor_emergency_kill() now sends SIGCONT before
  SIGTERM so the abort works on a STOPPED child too.
- src/tools/editor.c: _editor_exit_cb handles WIFSIGNALED cleanly with
  "Editor session cancelled." instead of printing garbage via
  WEXITSTATUS on a signaled child.
- src/profanity.c: SIGTSTP / SIGCONT signal handlers set flags consumed
  by the main loop. Editor STOPPED -> editor_emergency_kill (returns to
  the chat with the pre-alt+c readline buffer intact). Plain Ctrl-Z
  with no editor -> mutt-style drop the process group to the shell.
  SIGCONT -> refresh curses with clearok + ui_resize. While editor is
  active, swallow suspend_requested (race: vim's kill(0,SIGTSTP) beats
  waitpid seeing STOPPED — next poll catches it).
- src/tools/editor.h: declare editor_check_stopped and editor_is_active.

Verbose comment blocks across the branch tightened to one to two lines.
2026-05-31 14:08:03 +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
11 changed files with 196 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,12 @@ static void _connect_default(const char* const account);
pthread_mutex_t lock;
static gboolean force_quit = FALSE;
// Signal-driven flags consumed by the main loop. SIGTSTP and a STOPPED editor
// child → drop the process group to the shell (mutt-style). SIGCONT →
// refresh curses on return.
static volatile sig_atomic_t suspend_requested = FALSE;
static volatile sig_atomic_t cont_requested = FALSE;
void
prof_run(gchar* log_level, gchar* account_name, gchar* config_file, gchar* log_file, gchar* theme_name, gchar** commands)
{
@@ -93,6 +99,36 @@ prof_run(gchar* log_level, gchar* account_name, gchar* config_file, gchar* log_f
int waittime = 0;
while (cont && !force_quit) {
log_stderr_handler();
// Resume from fg after SIGSTOP. Vim handles its own refresh.
if (cont_requested) {
cont_requested = FALSE;
if (!editor_is_active()) {
clearok(stdscr, TRUE);
ui_resize();
}
}
// Editor STOPPED → abort. Plain Ctrl-Z → drop to shell. While editor
// is active, swallow suspend_requested (race: vim's kill(0,SIGTSTP)
// beats waitpid seeing STOPPED) — next poll catches it.
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);
}
}
session_check_autoaway();
line = commands ? *commands : inp_readline();
@@ -182,6 +218,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 +237,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,17 @@ typedef struct EditorContext
void* user_data;
} EditorContext;
// Re-entrancy guard (lost in the async-editor rewrite). Prevents a plugin
// from spawning a second editor that would fight the first for the terminal.
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;
@@ -52,8 +60,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 +80,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 +133,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 +145,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 +156,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 +169,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,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 the editor child watch dispatch — otherwise a prompt raised
// mid-editor-session deadlocks here.
while (g_main_context_iteration(NULL, FALSE))
;
}
status_bar_clear_prompt();
return line;
@@ -338,12 +346,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 the editor child watch dispatch — otherwise a prompt raised
// mid-editor-session deadlocks here.
while (g_main_context_iteration(NULL, FALSE))
;
}
get_password = FALSE;
status_bar_clear_prompt();
@@ -459,7 +471,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)
{
}