Skip to content

Commit 29038b3

Browse files
authored
fix(ci): match E2E app tags literally, make them unique per run, and never stop the caller (#8721)
* fix(ci): match E2E app tags literally, make them unique per run, and never stop the caller - stop-session.sh matches `E2E_APP=<tag>` as a fixed string, so a tag like `s.im` or a prefix like `sc` no longer matches another app's `E2E_APP=scim` - each HTTP end-to-end app's tag is `<app>-<run id>-<attempt>-<shell pid>`, so it is unique among the runner user's processes - the script never signals itself, its ancestors or its own subshells, even when the caller exported the same tag; /proc is read without forking so scans stay cheap - a surviving Next telemetry flush is labelled as expected, so an unexpected escapee stands out * fix(ci): never spare an app process whose ancestry changes mid-scan, and confirm an empty app on two scans
1 parent 988b079 commit 29038b3

2 files changed

Lines changed: 79 additions & 19 deletions

File tree

‎.github/scripts/stop-session.sh‎

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33
#
44
# Usage: stop-session.sh <session-leader-pid> <app-tag>
55
#
6-
# Start the app as `E2E_APP=<app-tag> setsid <command> &`. Its processes are the members of the
7-
# leader's session, plus any process that inherited `E2E_APP=<app-tag>` but left the session.
6+
# Start the app as `E2E_APP=<app-tag> setsid <command> &`, with a tag no other process of this
7+
# user carries (CI uses `<app>-<run id>-<attempt>-<shell pid>`). Its processes are the members of
8+
# the leader's session, plus any process whose environment holds exactly `E2E_APP=<app-tag>` but
9+
# left the session. This script, its ancestors and its own children are never signalled, even
10+
# when they carry the tag.
811
# `next dev` needs both: its server and workers stay in the session, while its telemetry flush is
912
# spawned detached, in a session of its own, and writes .next/dev/trace after `next dev` exits.
1013
# The next app in the job wipes that directory, so it must not start while any of them runs.
@@ -20,11 +23,55 @@ leader=$1
2023
tag=$2
2124
grace_seconds=10
2225

26+
# Sets `state` and `parent` from /proc/<pid>/stat without forking, so a scan stays cheap, and
27+
# fails once the process is gone. The command name can hold spaces and parentheses, so the
28+
# fields are read after its last ")".
29+
read_stat() {
30+
local stat rest
31+
read -r stat < "/proc/$1/stat" 2>/dev/null || return 1
32+
rest=${stat##*") "}
33+
state=${rest%% *}
34+
rest=${rest#* }
35+
parent=${rest%% *}
36+
}
37+
38+
ancestors=" "
39+
pid=$$
40+
while read_stat "$pid" && [ "$parent" -gt 0 ]; do
41+
ancestors+="$parent "
42+
pid=$parent
43+
[ "$pid" -eq 1 ] && break
44+
done
45+
46+
# This script, an ancestor of it, or one of its own subshells and commands, which inherit the tag
47+
# when the caller exported it. When a process on the walk exits mid-walk, the candidate has been
48+
# reparented, so the walk restarts from it; ancestry that still won't resolve counts as not ours,
49+
# so an app process is never spared by accident.
50+
is_own() {
51+
local candidate=$1 pid attempt
52+
[[ "$ancestors" == *" $candidate "* ]] && return 0
53+
for attempt in 1 2 3; do
54+
pid=$candidate
55+
while [ "$pid" -gt 1 ]; do
56+
[ "$pid" -eq $$ ] && return 0
57+
read_stat "$pid" || continue 2
58+
pid=$parent
59+
done
60+
return 1
61+
done
62+
return 1
63+
}
64+
2365
app_pids() {
66+
local pid
2467
{
25-
ps -s "$leader" -o pid=,stat= 2>/dev/null | awk '$2 !~ /^Z/ { print $1 }'
26-
grep -lzx "E2E_APP=$tag" /proc/[0-9]*/environ 2>/dev/null | cut -d/ -f3
27-
} | sort -u
68+
ps -s "$leader" -o pid= 2>/dev/null
69+
grep -Flzx "E2E_APP=$tag" /proc/[0-9]*/environ 2>/dev/null | cut -d/ -f3
70+
} | sort -u | while read -r pid; do
71+
read_stat "$pid" || continue
72+
[ "$state" = Z ] && continue
73+
is_own "$pid" || echo "$pid"
74+
done
2875
}
2976

3077
signal_app() {
@@ -44,7 +91,10 @@ leader_running() {
4491
list_app() {
4592
local pids
4693
pids=$(app_pids | paste -sd, -)
47-
[ -z "$pids" ] || ps -p "$pids" -o pid,sid,stat,etimes,args 2>/dev/null | cut -c1-200 || true
94+
[ -z "$pids" ] && return
95+
ps -p "$pids" -o pid,sid,stat,etimes,args 2>/dev/null |
96+
awk '/next\/dist\/telemetry\/detached-flush\.js/ { print "(expected telemetry flush) " $0; next } { print }' |
97+
cut -c1-220 || true
4898
}
4999

50100
# Centiseconds since boot: monotonic, and independent of the locale's decimal separator.
@@ -54,14 +104,20 @@ now_cs() {
54104
echo "${uptime/./}"
55105
}
56106

57-
# Re-sends the signal every 0.1s while the check holds, until the shared deadline.
58-
# Succeeds once the check stops holding.
107+
# Re-sends the signal every 0.1s while the check holds, until the shared deadline. Succeeds
108+
# once the check has stopped holding on two scans 0.1s apart, so a process missed by one scan
109+
# (spawned, or mid-reparenting, while it ran) is still caught.
59110
signal_while() {
60-
local signal=$1
111+
local signal=$1 clear=0
61112
shift
62113
while ((10#$(now_cs) < deadline)); do
63-
"$@" || return 0
64-
signal_app "$signal"
114+
if "$@"; then
115+
clear=0
116+
signal_app "$signal"
117+
else
118+
clear=$((clear + 1))
119+
((clear >= 2)) && return 0
120+
fi
65121
sleep 0.1
66122
done
67123
! "$@"

‎.github/workflows/test-build.yml‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -242,11 +242,12 @@ jobs:
242242
server_log="$report_dir/scim-next.log"
243243
mkdir -p "$report_dir"
244244
rm -rf .next/dev
245-
E2E_APP=scim setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3017 > "$server_log" 2>&1 &
245+
app_tag="scim-$GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT-$$"
246+
E2E_APP="$app_tag" setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3017 > "$server_log" 2>&1 &
246247
server_pid=$!
247248
finish() {
248249
status=$?
249-
bash ../../.github/scripts/stop-session.sh "$server_pid" scim || status=1
250+
bash ../../.github/scripts/stop-session.sh "$server_pid" "$app_tag" || status=1
250251
wait "$server_pid" 2>/dev/null || true
251252
awk '/^ (GET|POST|PUT|PATCH|DELETE|HEAD) \/api\// { print }' "$server_log" > "$report_dir/scim-http-status.log"
252253
if [ "$status" -ne 0 ]; then
@@ -295,11 +296,12 @@ jobs:
295296
server_log="$report_dir/cli-next.log"
296297
mkdir -p "$report_dir"
297298
rm -rf .next/dev
298-
E2E_APP=cli setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3018 > "$server_log" 2>&1 &
299+
app_tag="cli-$GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT-$$"
300+
E2E_APP="$app_tag" setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3018 > "$server_log" 2>&1 &
299301
server_pid=$!
300302
finish() {
301303
status=$?
302-
bash ../../.github/scripts/stop-session.sh "$server_pid" cli || status=1
304+
bash ../../.github/scripts/stop-session.sh "$server_pid" "$app_tag" || status=1
303305
wait "$server_pid" 2>/dev/null || true
304306
if [ "$status" -ne 0 ]; then
305307
tail -n 200 "$server_log"
@@ -346,11 +348,12 @@ jobs:
346348
server_log="$report_dir/stop-after-next.log"
347349
mkdir -p "$report_dir"
348350
rm -rf .next/dev
349-
E2E_APP=stop-after setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3018 > "$server_log" 2>&1 &
351+
app_tag="stop-after-$GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT-$$"
352+
E2E_APP="$app_tag" setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3018 > "$server_log" 2>&1 &
350353
server_pid=$!
351354
finish() {
352355
status=$?
353-
bash ../../.github/scripts/stop-session.sh "$server_pid" stop-after || status=1
356+
bash ../../.github/scripts/stop-session.sh "$server_pid" "$app_tag" || status=1
354357
wait "$server_pid" 2>/dev/null || true
355358
awk '/^ (GET|POST|PUT|PATCH|DELETE|HEAD) \/api\// { print }' "$server_log" > "$report_dir/stop-after-http-status.log"
356359
if [ "$status" -ne 0 ]; then
@@ -397,11 +400,12 @@ jobs:
397400
server_log="$report_dir/desktop-inbox-next.log"
398401
mkdir -p "$report_dir"
399402
rm -rf .next/dev
400-
E2E_APP=desktop-inbox setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3019 > "$server_log" 2>&1 &
403+
app_tag="desktop-inbox-$GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT-$$"
404+
E2E_APP="$app_tag" setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3019 > "$server_log" 2>&1 &
401405
server_pid=$!
402406
finish() {
403407
status=$?
404-
bash ../../.github/scripts/stop-session.sh "$server_pid" desktop-inbox || status=1
408+
bash ../../.github/scripts/stop-session.sh "$server_pid" "$app_tag" || status=1
405409
wait "$server_pid" 2>/dev/null || true
406410
awk '/^ (GET|POST|PUT|PATCH|DELETE|HEAD) \/api\// { print }' "$server_log" > "$report_dir/desktop-inbox-http-status.log"
407411
if [ "$status" -ne 0 ]; then

0 commit comments

Comments
 (0)