Queue control mode notifications rather than emitting them inside

%begin/%end, GitHub issue 5458 from George Nachman.
2026-08-24 07:31:36 +00:00 · 2026-08-03 13:38:42 +00:00
parent 01a775a46c
commit 6db5175e4e
5 changed files with 183 additions and 77 deletions
--- a/cfg.c
+++ b/cfg.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: cfg.c,v 1.90 2026/07/14 17:17:17 nicm Exp $ */
+/* $OpenBSD: cfg.c,v 1.91 2026/08/03 13:38:42 nicm Exp $ */

 /*
 * Copyright (c) 2008 Nicholas Marriott <nicholas.marriott@gmail.com>
@@ -224,13 +224,15 @@ cfg_print_causes(struct cmdq_item *item)
 {
 	struct client	*c = cmdq_get_client(item);
 	u_int		 i;
+	char		*cause;

 	for (i = 0; i < cfg_ncauses; i++) {
+		cause = cfg_causes[i];
 		if (c != NULL && (c->flags & CLIENT_CONTROL))
-			control_write(c, "%%config-error %s", cfg_causes[i]);
+			control_notify_write(c, "%%config-error %s", cause);
 		else
-			cmdq_print(item, "%s", cfg_causes[i]);
-		free(cfg_causes[i]);
+			cmdq_print(item, "%s", cause);
+		free(cause);
 	}

 	free(cfg_causes);
@@ -245,14 +247,16 @@ cfg_show_causes(struct session *s)
 	struct window_pane		*wp;
 	struct window_mode_entry	*wme;
 	u_int				 i;
+	char				*cause;

 	if (cfg_ncauses == 0)
 		return;

 	if (c != NULL && (c->flags & CLIENT_CONTROL)) {
 		for (i = 0; i < cfg_ncauses; i++) {
-			control_write(c, "%%config-error %s", cfg_causes[i]);
-			free(cfg_causes[i]);
+			cause = cfg_causes[i];
+			control_notify_write(c, "%%config-error %s", cause);
+			free(cause);
 		}
 		goto out;
 	}
--- a/cmd-queue.c
+++ b/cmd-queue.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: cmd-queue.c,v 1.121 2026/07/14 17:17:17 nicm Exp $ */
+/* $OpenBSD: cmd-queue.c,v 1.122 2026/08/03 13:38:42 nicm Exp $ */

 /*
 * Copyright (c) 2013 Nicholas Marriott <nicholas.marriott@gmail.com>
@@ -803,7 +803,7 @@ cmdq_guard(struct cmdq_item *item, const char *guard, int flags)
 	u_int		 number = item->number;

 	if (c != NULL && (c->flags & CLIENT_CONTROL))
-		control_write(c, "%%%s %ld %u %d", guard, t, number, flags);
+		control_write_guard(c, guard, t, number, flags);
 }

 /* Show message from command. */
--- a/control-notify.c
+++ b/control-notify.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: control-notify.c,v 1.37 2026/07/10 13:38:45 nicm Exp $ */
+/* $OpenBSD: control-notify.c,v 1.38 2026/08/03 13:38:42 nicm Exp $ */

 /*
 * Copyright (c) 2012 Nicholas Marriott <nicholas.marriott@gmail.com>
@@ -44,7 +44,8 @@ control_pane_mode_changed_cb(__unused const char *name,
 		TAILQ_FOREACH(c, &clients, entry) {
 			if (!CONTROL_SHOULD_NOTIFY_CLIENT(c))
 				continue;
-			control_write(c, "%%pane-mode-changed %%%u", wp->id);
+			control_notify_write(c, "%%pane-mode-changed %%%u",
+			    wp->id);
 		}
 		return;
 	}
@@ -53,8 +54,9 @@ control_pane_mode_changed_cb(__unused const char *name,
 	if (value == NULL)
 		return;
 	TAILQ_FOREACH(c, &clients, entry) {
-		if (CONTROL_SHOULD_NOTIFY_CLIENT(c))
-			control_write(c, "%%pane-mode-changed %s", value);
+		if (!CONTROL_SHOULD_NOTIFY_CLIENT(c))
+			continue;
+		control_notify_write(c, "%%pane-mode-changed %s", value);
 	}
 	free(value);
 }
@@ -92,7 +94,7 @@ control_window_layout_changed_cb(__unused const char *name,
 			continue;
 		s = c->session;
 		if (winlink_find_by_window_id(&s->windows, w->id) != NULL)
-			control_write(c, "%s", cp);
+			control_notify_write(c, "%s", cp);
 	}
 	free(cp);
 }
@@ -111,7 +113,7 @@ control_window_pane_changed_cb(__unused const char *name,
 		if (!CONTROL_SHOULD_NOTIFY_CLIENT(c))
 			continue;

-		control_write(c, "%%window-pane-changed @%u %%%u", w->id,
+		control_notify_write(c, "%%window-pane-changed @%u %%%u", w->id,
 		    w->active->id);
 	}
 }
@@ -133,9 +135,11 @@ control_window_unlinked_cb(__unused const char *name, struct event_payload *ep,
 		cs = c->session;

 		if (winlink_find_by_window_id(&cs->windows, w->id) != NULL)
-			control_write(c, "%%window-close @%u", w->id);
-		else
-			control_write(c, "%%unlinked-window-close @%u", w->id);
+			control_notify_write(c, "%%window-close @%u", w->id);
+		else {
+			control_notify_write(c, "%%unlinked-window-close @%u",
+			    w->id);
+		}
 	}
 }

@@ -156,9 +160,11 @@ control_window_linked_cb(__unused const char *name, struct event_payload *ep,
 		cs = c->session;

 		if (winlink_find_by_window_id(&cs->windows, w->id) != NULL)
-			control_write(c, "%%window-add @%u", w->id);
-		else
-			control_write(c, "%%unlinked-window-add @%u", w->id);
+			control_notify_write(c, "%%window-add @%u", w->id);
+		else {
+			control_notify_write(c, "%%unlinked-window-add @%u",
+			    w->id);
+		}
 	}
 }

@@ -179,11 +185,11 @@ control_window_renamed_cb(__unused const char *name, struct event_payload *ep,
 		cs = c->session;

 		if (winlink_find_by_window_id(&cs->windows, w->id) != NULL) {
-			control_write(c, "%%window-renamed @%u %s", w->id,
-			    w->name);
-		} else {
-			control_write(c, "%%unlinked-window-renamed @%u %s",
+			control_notify_write(c, "%%window-renamed @%u %s",
 			    w->id, w->name);
+		} else {
+			control_notify_write(c,
+			    "%%unlinked-window-renamed @%u %s", w->id, w->name);
 		}
 	}
 }
@@ -206,11 +212,12 @@ control_client_session_changed_cb(__unused const char *name,
 			continue;

 		if (cc == c) {
-			control_write(c, "%%session-changed $%u %s", s->id,
-			    s->name);
+			control_notify_write(c, "%%session-changed $%u %s",
+			    s->id, s->name);
 		} else {
-			control_write(c, "%%client-session-changed %s $%u %s",
-			    cc->name, s->id, s->name);
+			control_notify_write(c,
+			    "%%client-session-changed %s $%u %s", cc->name,
+			    s->id, s->name);
 		}
 	}
 }
@@ -226,8 +233,9 @@ control_client_detached_cb(__unused const char *name, struct event_payload *ep,
 	if (cc == NULL)
 		return;
 	TAILQ_FOREACH(c, &clients, entry) {
-		if (CONTROL_SHOULD_NOTIFY_CLIENT(c))
-			control_write(c, "%%client-detached %s", cc->name);
+		if (!CONTROL_SHOULD_NOTIFY_CLIENT(c))
+			continue;
+		control_notify_write(c, "%%client-detached %s", cc->name);
 	}
 }

@@ -244,8 +252,8 @@ control_session_renamed_cb(__unused const char *name, struct event_payload *ep,
 	TAILQ_FOREACH(c, &clients, entry) {
 		if (!CONTROL_SHOULD_NOTIFY_CLIENT(c))
 			continue;
-
-		control_write(c, "%%session-renamed $%u %s", s->id, s->name);
+		control_notify_write(c, "%%session-renamed $%u %s", s->id,
+		    s->name);
 	}
 }

@@ -259,8 +267,7 @@ control_session_created_cb(__unused const char *name,
 	TAILQ_FOREACH(c, &clients, entry) {
 		if (!CONTROL_SHOULD_NOTIFY_CLIENT(c))
 			continue;
-
-		control_write(c, "%%sessions-changed");
+		control_notify_write(c, "%%sessions-changed");
 	}
 }

@@ -274,8 +281,7 @@ control_session_closed_cb(__unused const char *name,
 	TAILQ_FOREACH(c, &clients, entry) {
 		if (!CONTROL_SHOULD_NOTIFY_CLIENT(c))
 			continue;
-
-		control_write(c, "%%sessions-changed");
+		control_notify_write(c, "%%sessions-changed");
 	}
 }

@@ -298,9 +304,8 @@ control_session_window_changed_cb(__unused const char *name,
 	TAILQ_FOREACH(c, &clients, entry) {
 		if (!CONTROL_SHOULD_NOTIFY_CLIENT(c))
 			continue;
-
-		control_write(c, "%%session-window-changed $%u @%u", s->id,
-		    s->curw->window->id);
+		control_notify_write(c, "%%session-window-changed $%u @%u",
+		    s->id, s->curw->window->id);
 	}
 }

@@ -317,8 +322,7 @@ control_paste_buffer_changed_cb(__unused const char *name,
 	TAILQ_FOREACH(c, &clients, entry) {
 		if (!CONTROL_SHOULD_NOTIFY_CLIENT(c))
 			continue;
-
-		control_write(c, "%%paste-buffer-changed %s", pbname);
+		control_notify_write(c, "%%paste-buffer-changed %s", pbname);
 	}
 }

@@ -335,8 +339,7 @@ control_paste_buffer_deleted_cb(__unused const char *name,
 	TAILQ_FOREACH(c, &clients, entry) {
 		if (!CONTROL_SHOULD_NOTIFY_CLIENT(c))
 			continue;
-
-		control_write(c, "%%paste-buffer-deleted %s", pbname);
+		control_notify_write(c, "%%paste-buffer-deleted %s", pbname);
 	}
 }

--- a/control.c
+++ b/control.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: control.c,v 1.62 2026/07/17 08:37:29 nicm Exp $ */
+/* $OpenBSD: control.c,v 1.63 2026/08/03 13:38:42 nicm Exp $ */

 /*
 * Copyright (c) 2012 Nicholas Marriott <nicholas.marriott@gmail.com>
@@ -53,6 +53,17 @@ struct control_block {
 	TAILQ_ENTRY(control_block)	 all_entry;
 };

+/*
+ * A notification line deferred because it was generated while a command's
+ * %begin/%end guard block was open. Notifications must never appear inside a
+ * guard block, so they are held here and flushed once the block closes.
+ */
+struct control_line {
+	char				*line;
+
+	TAILQ_ENTRY(control_line)	 entry;
+};
+
 /* Control client pane. */
 struct control_pane {
 	u_int				 pane;
@@ -102,6 +113,13 @@ struct control_state {
 	struct bufferevent		*write_event;

 	struct monitor_set		*subs;
+
+	/*
+	 * Depth of open %begin/%end guard blocks and notifications deferred
+	 * until the outermost block closes.
+	 */
+	int				 guard_depth;
+	TAILQ_HEAD(, control_line)	 deferred;
 };

 /* Low and high watermarks. */
@@ -352,7 +370,7 @@ control_continue_pane(struct client *c, struct window_pane *wp)
 		cp->flags &= ~CONTROL_PANE_PAUSED;
 		memcpy(&cp->offset, &wp->offset, sizeof cp->offset);
 		memcpy(&cp->queued, &wp->offset, sizeof cp->queued);
-		control_write(c, "%%continue %%%u", wp->id);
+		control_notify_write(c, "%%continue %%%u", wp->id);
 	}
 }

@@ -366,52 +384,120 @@ control_pause_pane(struct client *c, struct window_pane *wp)
 	if (~cp->flags & CONTROL_PANE_PAUSED) {
 		cp->flags |= CONTROL_PANE_PAUSED;
 		control_discard_pane(c, cp);
-		control_write(c, "%%pause %%%u", wp->id);
+		control_notify_write(c, "%%pause %%%u", wp->id);
 	}
 }

-/* Write a line. */
-static void printflike(2, 0)
-control_vwrite(struct client *c, const char *fmt, va_list ap)
-{
-	struct control_state	*cs = c->control_state;
-	char			*s;
-
-	xvasprintf(&s, fmt, ap);
-	log_debug("%s: %s: writing line: %s", __func__, c->name, s);
-
-	bufferevent_write(cs->write_event, s, strlen(s));
-	bufferevent_write(cs->write_event, "\n", 1);
-
-	bufferevent_enable(cs->write_event, EV_WRITE);
-	free(s);
-}
-
-/* Write a line. */
-void
-control_write(struct client *c, const char *fmt, ...)
+/* Write an already-formatted line, queueing it behind %output if needed. */
+static void
+control_write_line(struct client *c, char *line)
 {
 	struct control_state	*cs = c->control_state;
 	struct control_block	*cb;
-	va_list			 ap;
-
-	va_start(ap, fmt);

 	if (TAILQ_EMPTY(&cs->all_blocks)) {
-		control_vwrite(c, fmt, ap);
-		va_end(ap);
+		log_debug("%s: %s: writing line: %s", __func__, c->name, line);
+		bufferevent_write(cs->write_event, line, strlen(line));
+		bufferevent_write(cs->write_event, "\n", 1);
+		bufferevent_enable(cs->write_event, EV_WRITE);
+		free(line);
 		return;
 	}

 	cb = xcalloc(1, sizeof *cb);
-	xvasprintf(&cb->line, fmt, ap);
+	cb->line = line;
 	TAILQ_INSERT_TAIL(&cs->all_blocks, cb, all_entry);
 	cb->t = get_timer();

 	log_debug("%s: %s: storing line: %s", __func__, c->name, cb->line);
 	bufferevent_enable(cs->write_event, EV_WRITE);
+}

+/* Flush notifications that were deferred while a guard block was open. */
+static void
+control_flush_deferred(struct client *c)
+{
+	struct control_state	*cs = c->control_state;
+	struct control_line	*cl, *cl1;
+
+	TAILQ_FOREACH_SAFE(cl, &cs->deferred, entry, cl1) {
+		TAILQ_REMOVE(&cs->deferred, cl, entry);
+		control_write_line(c, cl->line);
+		free(cl);
+	}
+}
+
+/*
+ * Write a line of command output or error text. This is a sink for arbitrary
+ * user-controlled text (command output, capture-pane, error messages), so it
+ * must never try to interpret the content: guard tracking is done only in
+ * control_write_guard.
+ */
+void
+control_write(struct client *c, const char *fmt, ...)
+{
+	va_list	 ap;
+	char	*line;
+
+	va_start(ap, fmt);
+	xvasprintf(&line, fmt, ap);
 	va_end(ap);
+
+	control_write_line(c, line);
+}
+
+/*
+ * Write a %begin, %end or %error guard around a command's output. This is the
+ * only place guard lines are produced, so the block depth is maintained here;
+ * when the outermost block closes any deferred notifications are flushed after
+ * it. "guard" is always one of the fixed strings from cmdq_guard, never user
+ * text.
+ */
+void
+control_write_guard(struct client *c, const char *guard, long t, u_int number,
+    int flags)
+{
+	struct control_state	*cs = c->control_state;
+	char			*line;
+
+	if (strcmp(guard, "begin") == 0)
+		cs->guard_depth++;
+
+	xasprintf(&line, "%%%s %ld %u %d", guard, t, number, flags);
+	control_write_line(c, line);
+
+	if (strcmp(guard, "begin") != 0 && cs->guard_depth > 0 &&
+	    --cs->guard_depth == 0)
+		control_flush_deferred(c);
+}
+
+/*
+ * Write a notification line. Notifications must never appear inside a command's
+ * %begin/%end guard block, so if one is open the line is deferred until it
+ * closes.
+ */
+void
+control_notify_write(struct client *c, const char *fmt, ...)
+{
+	struct control_state	*cs = c->control_state;
+	struct control_line	*cl;
+	va_list			 ap;
+	char			*line;
+
+	va_start(ap, fmt);
+	xvasprintf(&line, fmt, ap);
+	va_end(ap);
+
+	if (cs->guard_depth == 0) {
+		control_write_line(c, line);
+		return;
+	}
+
+	log_debug("%s: %s: deferring notification: %s", __func__, c->name,
+	    line);
+	cl = xcalloc(1, sizeof *cl);
+	cl->line = line;
+	TAILQ_INSERT_TAIL(&cs->deferred, cl, entry);
 }

 /* Check age for this pane. */
@@ -438,7 +524,7 @@ control_check_age(struct client *c, struct window_pane *wp,
 			return (0);
 		cp->flags |= CONTROL_PANE_PAUSED;
 		control_discard_pane(c, cp);
-		control_write(c, "%%pause %%%u", wp->id);
+		control_notify_write(c, "%%pause %%%u", wp->id);
 	} else {
 		if (age < CONTROL_MAXIMUM_AGE)
 			return (0);
@@ -805,14 +891,17 @@ control_sub_change(struct monitor_change *change, __unused void *data)

 	if (wp != NULL) {
 		w = wp->window;
-		control_write(c, "%%subscription-changed %s $%u @%u %u %%%u : %s",
+		control_notify_write(c,
+		    "%%subscription-changed %s $%u @%u %u %%%u : %s",
 		    change->name, s->id, w->id, wl->idx, wp->id, change->value);
 	} else if (wl != NULL) {
 		w = wl->window;
-		control_write(c, "%%subscription-changed %s $%u @%u %u - : %s",
+		control_notify_write(c,
+		    "%%subscription-changed %s $%u @%u %u - : %s",
 		    change->name, s->id, w->id, wl->idx, change->value);
 	} else {
-		control_write(c, "%%subscription-changed %s $%u - - - : %s",
+		control_notify_write(c,
+		    "%%subscription-changed %s $%u - - - : %s",
 		    change->name, s->id, change->value);
 	}
 }
@@ -835,6 +924,7 @@ control_start(struct client *c)
 	RB_INIT(&cs->windows);
 	TAILQ_INIT(&cs->pending_list);
 	TAILQ_INIT(&cs->all_blocks);
+	TAILQ_INIT(&cs->deferred);
 	cs->subs = monitor_create_client(c, control_sub_change, NULL);

 	cs->read_event = bufferevent_new(c->fd, control_read_callback,
@@ -885,12 +975,19 @@ control_stop(struct client *c)
 	struct control_state	*cs = c->control_state;
 	struct control_block	*cb, *cb1;
 	struct control_window	*cw, *cw1;
+	struct control_line	*cl, *cl1;

 	if (cs == NULL)
 		return;

 	monitor_destroy(cs->subs);

+	TAILQ_FOREACH_SAFE(cl, &cs->deferred, entry, cl1) {
+		TAILQ_REMOVE(&cs->deferred, cl, entry);
+		free(cl->line);
+		free(cl);
+	}
+
 	if (~c->flags & CLIENT_CONTROLCONTROL)
 		bufferevent_free(cs->write_event);
 	bufferevent_free(cs->read_event);
--- a/tmux.h
+++ b/tmux.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: tmux.h,v 1.1419 2026/08/03 12:58:53 nicm Exp $ */
+/* $OpenBSD: tmux.h,v 1.1420 2026/08/03 13:38:42 nicm Exp $ */

 /*
 * Copyright (c) 2007 Nicholas Marriott <nicholas.marriott@gmail.com>
@@ -3985,6 +3985,8 @@ struct window_pane_offset *control_pane_offset(struct client *,
 	   struct window_pane *, int *);
 void	control_reset_offsets(struct client *);
 void printflike(2, 3) control_write(struct client *, const char *, ...);
+void printflike(2, 3) control_notify_write(struct client *, const char *, ...);
+void	control_write_guard(struct client *, const char *, long, u_int, int);
 void	control_write_output(struct client *, struct window_pane *);
 int	control_all_done(struct client *);
 void	control_add_sub(struct client *, const char *, enum monitor_type, int,