screen-redraw: don't let one client's pane title leak to another

redraw_damage_refresh_status() force-regenerates a pane's border-status
title when a damage rectangle touches it (window_make_pane_status()'s
own content-diff cache can't tell "physically disturbed" from "never
changed"), guarded by the per-pane PANE_NEWSTATUS flag. But the
rendered content is per-client - window_make_pane_status() formats
pane-border-format using the requesting client's own context, so
fields like #{client_name} genuinely differ per client - while
wp->status_screen/PANE_NEWSTATUS are shared by every client viewing
the pane. With two clients attached to the same session, whichever
client's damage pass ran first rendered its own text and set the flag;
every other client's damage pass in that tick, or any later one, since
nothing else clears the flag on this path, found it already set and
skipped rendering - silently reusing the first client's text.

Fix: a per-pass serial (redraw_status_serial, bumped once per
redraw_client_damage() call - one call is one client's one redraw
pass) instead of a sticky flag. Deduplicates repeated calls within the
same client's same pass exactly as before, but forces a fresh,
correctly-client-formatted render whenever a different client or a
later pass touches the same pane's status - without needing to track
and later invalidate a client pointer with its own lifetime.

Also added a permanent log_debug() line for the actual regenerate,
since this class of bug (a damage pass silently trusting stale
per-pane state that should have been per-client) is otherwise
invisible to any external capture: an unrelated periodic per-client
status refresh reliably repaints each client's title correctly again
within the very same tick, before anything is ever flushed to either
terminal, so the wrong content this bug produces was never actually
observable in a capture-pane-based test - confirmed by direct
instrumentation while building the regression test below, which is
exactly why the fix is verified via this log rather than a capture.

regress/floating-pane-status-cross-client.sh attaches two clients to
one session, each with its own pane-border-format referencing
#{client_name}, and triggers a damage-only palette update (OSC 4) in
a tiled pane whose geometry overlaps a floating pane's own
border-status row - carefully picked so the trigger has no side
effect that would otherwise force a normal, already-correct, full
per-client status re-render in the same pass, which would mask the
result either way. Verified failing 3/3 against the pre-fix code
(neither client's damage pass regenerates at all - both silently
reuse whatever a much earlier full redraw left in the shared buffer)
and passing 5/5 against the fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Michael Grant
2026-09-22 16:13:53 +01:00
parent 104a0cee99
commit 04dbc6d89b
3 changed files with 141 additions and 1 deletions

View File

@@ -0,0 +1,117 @@
#!/bin/sh
# redraw_damage_refresh_status() (screen-redraw.c) force-regenerates a
# pane's border-status title when a damage rectangle touches it, guarded
# by the per-pane PANE_NEWSTATUS flag. window_make_pane_status() formats
# pane-border-format using the requesting client's own context (so e.g.
# #{client_name} differs per client), but wp->status_screen/PANE_NEWSTATUS
# are shared by every client viewing the pane. With two clients attached
# to the same session, whichever client's damage pass runs first renders
# its own text and sets the flag; the other client's damage pass, finding
# the flag already set, used to skip rendering entirely and reuse
# whatever was already there.
#
# This checks the actual server-side decision via the -vv log rather than
# a visual capture: an unrelated periodic client status-refresh reliably
# repaints each client's title correctly again within the same tick right
# after the buggy decision is made, before anything is ever flushed to
# either terminal, so the wrong content this bug produces is never
# visible to any external capture - the log is the only place the actual
# bug (or its absence) can be observed.
#
# A floating pane with its own pane-border-status is positioned so that a
# damage rectangle from an *unrelated* palette change (OSC 4) in the
# underlying tiled pane - whose own geometry spans the whole window -
# overlaps the floating pane's title row without touching its content,
# giving a damage-only trigger with no side effect that would otherwise
# force a normal (non-buggy) full per-client status re-render in the same
# pass and mask the result either way.
PATH=/bin:/usr/bin
TERM=screen
LC_ALL=C.UTF-8
export PATH TERM LC_ALL
[ -z "$TEST_TMUX" ] && TEST_TMUX=$(readlink -f ../tmux)
DIR=$(mktemp -d) || exit 1
cd "$DIR" || exit 1
INNER="$TEST_TMUX -vv -Lstatuscc-inner-$$ -f/dev/null"
OUTER1="$TEST_TMUX -Lstatuscc-outer1-$$ -f/dev/null"
OUTER2="$TEST_TMUX -Lstatuscc-outer2-$$ -f/dev/null"
fail()
{
echo "$*" >&2
exit 1
}
cleanup()
{
$OUTER1 kill-server 2>/dev/null
$OUTER2 kill-server 2>/dev/null
$INNER kill-server 2>/dev/null
cd /
rm -rf "$DIR"
}
trap cleanup 0 1 15
BASEEMITTER=$DIR/base-emitter.pl
cat >"$BASEEMITTER" <<'PERL'
use strict;
use warnings;
$| = 1;
my $line = <STDIN>;
print "\e]4;1;rgb:11/22/33\e\\";
sleep 100;
PERL
$INNER new-session -d -s inner -x 40 -y 10 "perl '$BASEEMITTER'" || exit 1
$INNER set-option -g status off || exit 1
$INNER set-option -g window-size manual || exit 1
$INNER set-option -g pane-border-status top || exit 1
$INNER set-option -g pane-border-format 'C=#{client_name}' || exit 1
BASE=$($INNER list-panes -t inner -F '#{pane_id}') || exit 1
FLOAT=$($INNER new-pane -d -PF '#{pane_id}' -x 20 -y 3 -X 5 -Y 4 \
'sleep 100') || exit 1
$OUTER1 new-session -d -s outer -x 40 -y 10 'sleep 100' || exit 1
$OUTER1 set-option -g status off || exit 1
$OUTER1 set-option -g window-size manual || exit 1
$OUTER1 set-option -g default-terminal screen-256color || exit 1
$OUTER1 respawn-pane -k -t outer:0.0 \
"$TEST_TMUX -Lstatuscc-inner-$$ -f/dev/null attach-session -t inner" ||
exit 1
sleep 0.5
NAME1=$($INNER list-clients -F '#{client_name}') || exit 1
$OUTER2 new-session -d -s outer -x 40 -y 10 'sleep 100' || exit 1
$OUTER2 set-option -g status off || exit 1
$OUTER2 set-option -g window-size manual || exit 1
$OUTER2 set-option -g default-terminal screen-256color || exit 1
$OUTER2 respawn-pane -k -t outer:0.0 \
"$TEST_TMUX -Lstatuscc-inner-$$ -f/dev/null attach-session -t inner" ||
exit 1
sleep 0.5
ALLNAMES=$($INNER list-clients -F '#{client_name}') || exit 1
NAME2=$(echo "$ALLNAMES" | grep -v "^$NAME1\$")
[ -n "$NAME2" ] || fail "sanity: could not identify the second client"
# Let any attach-driven full redraw (and its own, non-buggy, per-client
# status render) finish completely before triggering the damage-only
# palette update.
sleep 1.5
$INNER send-keys -t "$BASE" Enter || exit 1
sleep 0.5
LOG=$(ls tmux-server*.log 2>/dev/null | head -1)
[ -n "$LOG" ] || fail "sanity: no server -vv log was produced"
n1=$(grep -c "regenerated pane .* status for $NAME1\$" "$LOG")
n2=$(grep -c "regenerated pane .* status for $NAME2\$" "$LOG")
[ "$n1" -ge 1 ] || fail "damage pass never regenerated $NAME1's own status - it reused whatever the other client's render left behind"
[ "$n2" -ge 1 ] || fail "damage pass never regenerated $NAME2's own status - it reused whatever the other client's render left behind"
exit 0

View File

@@ -228,6 +228,12 @@ struct redraw_build_cell {
static struct redraw_build_cell *redraw_cells;
static size_t redraw_ncells;
/*
* Bumped once per redraw_client_damage() call (one client's one redraw
* pass) - see redraw_damage_refresh_status().
*/
static u_int redraw_status_serial;
/* Context for building the scene. */
struct redraw_build_ctx {
struct client *c;
@@ -2031,6 +2037,17 @@ redraw_pane_scrollbar(struct client *c, struct window_pane *wp)
* whenever PANE_NEWSTATUS is not set, leaving a pane's border-status title
* blank until some unrelated redraw happens to touch it (e.g. a focus
* change or window resize).
*
* wp->status_screen/PANE_NEWSTATUS are per-pane, but the formatted content
* (window_make_pane_status() expands pane-border-format, which can read
* per-client fields like #{client_name}) is per-client. Gating purely on
* PANE_NEWSTATUS would let one client's damage pass render its own text,
* set the flag, and leave every other client's pass - this tick or any
* later one, since nothing else clears it here - reusing that stale,
* wrong-client text. redraw_status_serial (bumped once per
* redraw_client_damage() call, i.e. once per client per pass) still
* dedupes repeat calls within that same pass, but forces a fresh,
* correctly-client-formatted render on every distinct client/pass.
*/
static void
redraw_damage_refresh_status(struct redraw_draw_ctx *dctx,
@@ -2039,13 +2056,17 @@ redraw_damage_refresh_status(struct redraw_draw_ctx *dctx,
struct redraw_span *first;
u_int width;
if (wp->flags & PANE_NEWSTATUS)
if ((wp->flags & PANE_NEWSTATUS) &&
wp->status_serial == redraw_status_serial)
return;
width = redraw_pane_status_width(dctx, wp, &first);
if (width == 0)
return;
log_debug("%s: regenerated pane %%%u status for %s", __func__, wp->id,
dctx->scene->c->name);
window_make_pane_status(wp, dctx->scene->c, width, first);
wp->flags |= PANE_NEWSTATUS;
wp->status_serial = redraw_status_serial;
}
/*
@@ -2231,6 +2252,7 @@ redraw_client_damage(struct client *c)
if (TAILQ_EMPTY(&w->damage))
return;
redraw_status_serial++;
scene = redraw_get_scene(c);
if (scene == NULL)

1
tmux.h
View File

@@ -1401,6 +1401,7 @@ struct window_pane {
struct screen base;
struct screen status_screen;
u_int status_serial;
TAILQ_HEAD(, window_mode_entry) modes;