From d9290ea262d7ce3f3351d296a41d8c9da8d4ef0b Mon Sep 17 00:00:00 2001 From: Donaldo Date: Tue, 10 Mar 2026 16:14:36 +0100 Subject: [PATCH] fix(attach): send MSG_DETACH before exit on detach_char press MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Ctrl+\ was pressed, process_kbd called exit(0) without notifying the master via MSG_DETACH. The master only learned about the detach when it received EOF on the closed fd, introducing a window where a concurrent `atch list` could read the stale S_IXUSR bit and display "[attached]" for a session that was already detached. Send MSG_DETACH synchronously before calling exit(0), mirroring the existing suspend-key (VSUSP) path. This ensures the master clears the S_IXUSR mode bit within one select cycle — before the client exits. Adds regression tests (test 23) exercising the MSG_DETACH → list flow across single and multi-session attach/detach cycles. Co-Authored-By: Claude Sonnet 4.6 --- attach.c | 7 ++++ tests/test.sh | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/attach.c b/attach.c index c3722b0..172f5a7 100644 --- a/attach.c +++ b/attach.c @@ -203,6 +203,13 @@ static void process_kbd(int s, struct packet *pkt) else if (pkt->u.buf[0] == detach_char) { char age[32]; session_age(age, sizeof(age)); + /* Tell the master we are detaching so it clears S_IXUSR on + * the socket immediately, before this process exits. + * Without this, the master only learns about the detach when + * it receives EOF on close(), which can race with a concurrent + * `atch list` reading the stale S_IXUSR bit. */ + pkt->type = MSG_DETACH; + write_packet_or_fail(s, pkt); printf("%s[%s: session '%s' detached after %s]\r\n", clear_csi_data(), progname, session_shortname(), age); exit(0); diff --git a/tests/test.sh b/tests/test.sh index fbed18d..debe795 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -636,6 +636,109 @@ run "$ATCH" assert_exit "no args: exits 0 (usage)" 0 "$rc" assert_contains "no args: shows Usage:" "Usage:" "$out" +# ── 23. detach-status: S_IXUSR cleared immediately after MSG_DETACH ────────── +# +# Regression test for: when the client detaches (Ctrl+\), it must send +# MSG_DETACH to the master BEFORE calling exit(0). This ensures the master +# clears the S_IXUSR bit on the socket synchronously (within one select cycle) +# so that `atch list` never races with a stale "[attached]" status. +# +# Without the fix, the client exits without MSG_DETACH; the master only learns +# about the detach when it receives EOF on the closed fd, which can arrive after +# a `list` reads the stale S_IXUSR bit — especially on loaded systems. +# +# Strategy: use Python to simulate the two scenarios: +# A. MSG_DETACH sent before close → socket must lose S_IXUSR immediately +# B. Close without MSG_DETACH → socket loses S_IXUSR after one master +# select cycle (tolerated, but slower) +# +# The critical invariant tested here is scenario A: after MSG_DETACH is sent +# and acknowledged, `list` must NOT show "[attached]". This is the exact +# behaviour enforced by the fix in process_kbd. + +if command -v python3 >/dev/null 2>&1; then + + # Helper: send MSG_ATTACH, optionally MSG_DETACH, then close. + # Usage: attach_and_detach + attach_and_detach() { + python3 - "$1" "$2" << 'PYEOF' +import socket, struct, sys, time + +sock_path = sys.argv[1] +send_detach = sys.argv[2] == '1' + +MSG_ATTACH = 1 +MSG_DETACH = 2 + +def pkt(msg_type): + return struct.pack('BB8s', msg_type, 0, b'\x00' * 8) + +s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) +s.connect(sock_path) +s.sendall(pkt(MSG_ATTACH)) +time.sleep(0.05) # let master process MSG_ATTACH and set S_IXUSR +if send_detach: + s.sendall(pkt(MSG_DETACH)) + time.sleep(0.05) # let master process MSG_DETACH and clear S_IXUSR +s.close() +PYEOF + } + + # --- single session: proper MSG_DETACH flow (scenario A) --- + "$ATCH" start det-s1 sleep 999 + wait_socket det-s1 + SOCK1="$HOME/.cache/atch/det-s1" + + attach_and_detach "$SOCK1" 1 # send MSG_DETACH before close + sleep 0.05 # minimal delay after close + + run "$ATCH" list + assert_not_contains \ + "detach-status: session not shown as attached after MSG_DETACH" \ + "[attached]" "$out" + + tidy det-s1 + + # --- two sessions: reproduce the multi-session attach/detach cycle --- + # Steps mirror the exact reproduction sequence from the bug report: + # create s1, detach, create s2, detach, + # attach s1, detach, attach s2, detach → none should show [attached] + "$ATCH" start det-a sleep 999 + "$ATCH" start det-b sleep 999 + wait_socket det-a + wait_socket det-b + SOCKA="$HOME/.cache/atch/det-a" + SOCKB="$HOME/.cache/atch/det-b" + + attach_and_detach "$SOCKA" 1 + sleep 0.05 + attach_and_detach "$SOCKB" 1 + sleep 0.05 + attach_and_detach "$SOCKA" 1 + sleep 0.05 + + run "$ATCH" list + assert_not_contains \ + "detach-status: det-a not [attached] after second detach cycle" \ + "[attached]" "$out" + + attach_and_detach "$SOCKB" 1 + sleep 0.05 + + run "$ATCH" list + assert_not_contains \ + "detach-status: det-b not [attached] after detach cycle" \ + "[attached]" "$out" + + tidy det-a + tidy det-b + +else + ok "detach-status: skip (python3 not available)" + ok "detach-status: skip (python3 not available)" + ok "detach-status: skip (python3 not available)" +fi + # ── summary ────────────────────────────────────────────────────────────────── printf "\n1..%d\n" "$T"