From 980a0054d1533e2e1dcb9a8c90a1bbdc668b8a7d Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 30 Sep 2026 03:43:48 +0000 Subject: [PATCH] fix(tools): one xce-build.sh run per build dir at a time (bug-1604) Two xce-build.sh runs against the same build dir raced: one run's setup_build rm -rf hit files the other was writing ('Directory not empty'), or deleted a binary the other had just built and tested. xce-build.sh now takes an exclusive per-build-dir lock right after cd-ing into src/ and holds it until it exits; a second run against the same dir waits, runs against different dirs are unaffected. The lock (src/tools/build-lock.sh) is an mkdir lock holding the owner PID, outside the build dir, keyed by a hash of its absolute path; it needs only coreutils (the nix dev shell has no flock) and takes over a lock whose owner died. src/tools/test-build-lock.sh tests it. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/tools/build-lock.sh | 59 ++++++++++++++++++ src/tools/test-build-lock.sh | 116 +++++++++++++++++++++++++++++++++++ src/tools/xce-build.sh | 8 +++ 3 files changed, 183 insertions(+) create mode 100644 src/tools/build-lock.sh create mode 100644 src/tools/test-build-lock.sh diff --git a/src/tools/build-lock.sh b/src/tools/build-lock.sh new file mode 100644 index 00000000..59204503 --- /dev/null +++ b/src/tools/build-lock.sh @@ -0,0 +1,59 @@ +# build-lock.sh — exclusive per-build-dir lock for xce-build.sh (bug-1604). +# +# Sourced, not executed. Two xce-build.sh runs against the SAME build dir used +# to race: one run's `rm -rf $BUILD_DIR` hit files the other was writing +# ("Directory not empty") or silently deleted a binary the other had just +# built and tested. The lock serializes them for the whole run (wipe, setup, +# compile, tests); runs against DIFFERENT build dirs stay concurrent. +# +# The lock is a directory created with mkdir (atomic; needs nothing beyond +# coreutils, unlike flock(1), which the nix dev shell does not provide), +# holding the owner's PID. It lives outside the build dir -- the wipe would +# delete it -- under $XCE_BUILD_LOCK_ROOT (default $TMPDIR or /tmp), named +# after a hash of the build dir's absolute path, so separate checkouts never +# share a lock. A lock whose owner is no longer running (a build killed with +# SIGKILL) is taken over. +# +# build_lock_acquire blocks until held +# build_lock_release releases it (safe to call twice) +# +# Tunables: XCE_BUILD_LOCK_ROOT, XCE_BUILD_LOCK_POLL_SECS (default 2). + +BUILD_LOCK_DIR="" + +build_lock_path() { + local abs sum + abs="$(realpath -m -- "$1")" + sum="$(printf '%s' "$abs" | cksum | cut -d' ' -f1)" + printf '%s/xce-build-%s.lock' "${XCE_BUILD_LOCK_ROOT:-${TMPDIR:-/tmp}}" "$sum" +} + +build_lock_acquire() { + local lock owner waited=0 poll="${XCE_BUILD_LOCK_POLL_SECS:-2}" + + lock="$(build_lock_path "$1")" + while ! mkdir "$lock" 2>/dev/null; do + owner="$(cat "$lock/pid" 2>/dev/null || true)" + if [ -n "$owner" ] && ! kill -0 "$owner" 2>/dev/null; then + # The owner died holding it. If two waiters take over the same + # stale lock at once, mkdir below still lets only one of them in. + echo "=== Build lock for $1 was left by PID $owner, which is gone -- taking it over ===" >&2 + rm -rf -- "$lock" + continue + fi + if [ "$waited" = 0 ]; then + echo "=== Waiting for another xce-build.sh run (PID ${owner:-?}) to finish with $1 ... ===" >&2 + waited=1 + fi + sleep "$poll" + done + echo "${BASHPID:-$$}" > "$lock/pid" + BUILD_LOCK_DIR="$lock" +} + +build_lock_release() { + if [ -n "$BUILD_LOCK_DIR" ]; then + rm -rf -- "$BUILD_LOCK_DIR" + BUILD_LOCK_DIR="" + fi +} diff --git a/src/tools/test-build-lock.sh b/src/tools/test-build-lock.sh new file mode 100644 index 00000000..ed25e459 --- /dev/null +++ b/src/tools/test-build-lock.sh @@ -0,0 +1,116 @@ +#!/bin/bash +# test-build-lock.sh — bug-1604: concurrent xce-build.sh runs against the same +# build dir must not race on its rm -rf / meson setup. +# +# Exercises src/tools/build-lock.sh (sourced by xce-build.sh) directly, so no +# real build is needed: +# 1. a second acquire of the SAME build dir waits until the first releases +# 2. a lock left by a dead process is taken over, not waited on forever +# 3. DIFFERENT build dirs never block each other +# 4. xce-build.sh takes the lock before setup_build's rm -rf +# +# Usage: +# bash src/tools/test-build-lock.sh +set -u + +TOOLS=$(cd "$(dirname "$0")" && pwd) + +PASS=0 +FAIL=0 + +green() { printf '\033[32m%s\033[0m\n' "$*"; } +red() { printf '\033[31m%s\033[0m\n' "$*"; } + +pass() { green " PASS: $*"; PASS=$((PASS+1)); } +fail() { red " FAIL: $*"; FAIL=$((FAIL+1)); } + +WORK=$(mktemp -d) +trap 'rm -rf "$WORK"' EXIT +export XCE_BUILD_LOCK_ROOT="$WORK/locks" +export XCE_BUILD_LOCK_POLL_SECS=0.1 +mkdir -p "$XCE_BUILD_LOCK_ROOT" + +if [ ! -f "$TOOLS/build-lock.sh" ]; then + fail "src/tools/build-lock.sh does not exist" + echo "Results: ${PASS} passed, ${FAIL} failed" + exit 1 +fi + +# A holder: takes the lock on $1, records when it has it, holds it for $2 s, +# records the release, exits. +holder() { + ( + . "$TOOLS/build-lock.sh" + build_lock_acquire "$1" >/dev/null 2>&1 + echo "$3 got $(date +%s.%N)" >> "$WORK/events" + sleep "$2" + echo "$3 released $(date +%s.%N)" >> "$WORK/events" + build_lock_release + ) +} + +event_time() { awk -v w="$1 $2" '$1" "$2 == w {print $3}' "$WORK/events"; } + +echo "── 1. same build dir: the second acquire waits for the first ──" +: > "$WORK/events" +holder "$WORK/b1" 1 A & +sleep 0.3 +holder "$WORK/b1" 0 B & +wait +a_rel=$(event_time A released) +b_got=$(event_time B got) +if [ -n "$a_rel" ] && [ -n "$b_got" ] && \ + awk -v a="$a_rel" -v b="$b_got" 'BEGIN { exit !(b >= a) }'; then + pass "B got the lock only after A released it" +else + fail "B got the lock while A still held it (A released=$a_rel, B got=$b_got)" +fi + +echo "── 2. a lock left by a dead process is taken over ──" +: > "$WORK/events" +( + . "$TOOLS/build-lock.sh" + build_lock_acquire "$WORK/b2" >/dev/null 2>&1 + # exit WITHOUT releasing, like a build killed with SIGKILL +) +start=$(date +%s) +holder "$WORK/b2" 0 C & +cpid=$! +( sleep 10; kill "$cpid" 2>/dev/null ) & +watchdog=$! +wait "$cpid" 2>/dev/null +kill "$watchdog" 2>/dev/null +if [ -n "$(event_time C got)" ] && [ $(( $(date +%s) - start )) -lt 10 ]; then + pass "a stale lock from a dead owner was taken over" +else + fail "waited on a lock whose owner is dead" +fi + +echo "── 3. different build dirs do not block each other ──" +: > "$WORK/events" +holder "$WORK/b3" 2 D & +sleep 0.3 +start=$(date +%s.%N) +holder "$WORK/b4" 0 E & +wait +e_got=$(event_time E got) +if [ -n "$e_got" ] && awk -v s="$start" -v e="$e_got" 'BEGIN { exit !(e - s < 1.5) }'; then + pass "a lock on another build dir was granted immediately" +else + fail "a lock on another build dir waited for an unrelated holder" +fi + +echo "── 4. xce-build.sh takes the lock before setup_build's rm -rf ──" +lock_line=$(grep -n '^build_lock_acquire ' "$TOOLS/xce-build.sh" | head -1 | cut -d: -f1) +setup_line=$(grep -n '^setup_build() {' "$TOOLS/xce-build.sh" | head -1 | cut -d: -f1) +first_call=$(grep -n '^ *setup_build$' "$TOOLS/xce-build.sh" | head -1 | cut -d: -f1) +if grep -q 'build-lock.sh' "$TOOLS/xce-build.sh" && [ -n "$lock_line" ] && \ + [ -n "$first_call" ] && [ "$lock_line" -lt "$first_call" ]; then + pass "xce-build.sh acquires the build-dir lock (line $lock_line) before the first setup_build call (line $first_call)" +else + fail "xce-build.sh does not acquire the build-dir lock before setup_build (lock=$lock_line setup_build()=$setup_line first call=$first_call)" +fi + +echo "" +echo "Results: ${PASS} passed, ${FAIL} failed" +[ "$FAIL" -eq 0 ] diff --git a/src/tools/xce-build.sh b/src/tools/xce-build.sh index 131d6808..9b5dffca 100755 --- a/src/tools/xce-build.sh +++ b/src/tools/xce-build.sh @@ -168,6 +168,14 @@ esac cd "$SRC_DIR" +# bug-1604: one xce-build.sh run per build dir at a time. A concurrent run +# against the same dir raced on setup_build's rm -rf ("Directory not empty") +# or deleted a binary the other run had just built; it now waits here until +# this run exits. Runs against different build dirs are not affected. +. "$SCRIPT_DIR/build-lock.sh" +build_lock_acquire "$BUILD_DIR" +trap build_lock_release EXIT + # build-time-versioning task 2.1: compute commit_id/build_time once, common to # every profile, and append to MESON_OPTS below (before either `meson setup` # call in setup_build() runs). This feeds src/meson.options' commit_id/