Fix Homebrew source build inactivity timeouts

__tests__/brew.test.ts

@@ -0,0 +1,317 @@
+import {spawn, spawnSync} from 'child_process';
+import fs from 'fs';
+import os from 'os';
+import path from 'path';
+
+const brew = fs.readFileSync(
+  path.join(__dirname, '../src/scripts/tools/brew.sh'),
+  'utf8'
+);
+const describeUnix = process.platform === 'win32' ? describe.skip : describe;
+
+describeUnix('Homebrew inactivity watchdog', () => {
+  let root: string;
+  let fixture: string;
+  let buildScript: string;
+
+  const pids = () =>
+    fs.existsSync(path.join(root, 'pids'))
+      ? fs
+          .readFileSync(path.join(root, 'pids'), 'utf8')
+          .trim()
+          .split('\n')
+          .map(Number)
+      : [];
+
+  const running = (pid: number) => {
+    const result = spawnSync('ps', ['-p', String(pid), '-o', 'stat='], {
+      encoding: 'utf8'
+    });
+    return result.status === 0 && !result.stdout.trim().startsWith('Z');
+  };
+
+  const cleanup = () => {
+    for (const pid of pids().reverse()) {
+      try {
+        process.kill(pid, 'SIGKILL');
+      } catch {
+        // The watchdog may have already terminated this process.
+      }
+    }
+  };
+
+  beforeEach(() => {
+    root = fs.mkdtempSync(path.join(os.tmpdir(), 'setup-php-brew-test-'));
+    fixture = path.join(root, 'source.cjs');
+    buildScript = path.join(root, 'Homebrew', 'build.rb');
+    fs.mkdirSync(path.dirname(buildScript));
+    fs.symlinkSync(fixture, buildScript);
+    fs.writeFileSync(
+      fixture,
+      `const fs = require('fs');
+const {spawn, spawnSync} = require('child_process');
+const role = process.argv[2] || 'brew';
+const pidFile = process.env.TEST_ROOT + '/pids';
+if (role === 'brew') {
+  if (fs.existsSync(pidFile)) {
+    for (const pid of fs.readFileSync(pidFile, 'utf8').trim().split('\\n')) {
+      const state = spawnSync('ps', ['-p', pid, '-o', 'stat='], {encoding: 'utf8'});
+      if (state.status === 0 && !state.stdout.trim().startsWith('Z')) {
+        fs.appendFileSync(process.env.TEST_ROOT + '/overlap', pid + '\\n');
+      }
+    }
+  }
+  fs.appendFileSync(process.env.TEST_ROOT + '/attempts', 'attempt\\n');
+  process.stdout.write('==> make\\n');
+}
+fs.appendFileSync(pidFile, process.pid + '\\n');
+if (role === 'compiler') {
+  process.on('SIGTERM', () => {});
+  if (process.env.TEST_BUILD_DURATION) {
+    setTimeout(() => process.exit(0), Number(process.env.TEST_BUILD_DURATION));
+  }
+} else {
+  const script = role === 'brew' ? process.env.TEST_BUILD_SCRIPT : __filename;
+  const child = spawn(process.execPath, [script, role === 'brew' ? 'builder' : 'compiler'], {
+    detached: true,
+    stdio: process.env.TEST_BUILD_STDIO
+  });
+  child.on('exit', status => {
+    if (process.env.TEST_BUILD_DURATION) {
+      if (role === 'brew') fs.writeFileSync(process.env.TEST_ROOT + '/built', 'done');
+      const delay = role === 'brew' ? Number(process.env.TEST_AFTER_BUILD_DELAY || 0) : 0;
+      setTimeout(() => process.exit(status || 0), delay);
+    }
+  });
+  child.unref();
+}
+setInterval(() => {}, 1000);
+`
+    );
+  });
+
+  afterEach(() => {
+    cleanup();
+    fs.rmSync(root, {recursive: true, force: true});
+  });
+
+  const run = (script: string, env: NodeJS.ProcessEnv = {}) =>
+    new Promise<{status: number | null; stdout: string; stderr: string}>(
+      (resolve, reject) => {
+        const child = spawn('bash', ['-c', brew + '\n' + script], {
+          detached: true,
+          env: {
+            ...process.env,
+            TEST_NODE: process.execPath,
+            TEST_ROOT: root,
+            TEST_FIXTURE: fixture,
+            TEST_BUILD_SCRIPT: buildScript,
+            TEST_BUILD_STDIO: 'ignore',
+            SETUP_PHP_BREW_WATCHDOG: 'true',
+            SETUP_PHP_BREW_INACTIVITY_TIMEOUT: '1',
+            SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: '1',
+            SETUP_PHP_BREW_WATCHDOG_POLL: '0.05',
+            SETUP_PHP_BREW_RETRY_ATTEMPTS: '3',
+            ...env
+          }
+        });
+        let stdout = '';
+        let stderr = '';
+        child.stdout.on('data', data => (stdout += data));
+        child.stderr.on('data', data => (stderr += data));
+        const timer = setTimeout(() => {
+          cleanup();
+          if (child.pid) {
+            try {
+              process.kill(-child.pid, 'SIGKILL');
+            } catch {
+              // The shell may have just exited.
+            }
+          }
+          reject(new Error('Watchdog did not finish: ' + stderr));
+        }, 20000);
+        child.on('error', error => {
+          clearTimeout(timer);
+          reject(error);
+        });
+        child.on('close', status => {
+          clearTimeout(timer);
+          resolve({status, stdout, stderr});
+        });
+      }
+    );
+
+  it.each(['ignore', 'inherit'])(
+    'kills the complete source-build tree with %s stdio before returning',
+    async stdio => {
+      const result = await run(
+        'run_with_inactivity_watchdog "$TEST_NODE" "$TEST_FIXTURE"',
+        {TEST_BUILD_STDIO: stdio}
+      );
+      expect(result.status).toBe(124);
+      expect(pids()).toHaveLength(3);
+      expect(pids().filter(running)).toEqual([]);
+      expect(result.stderr).toContain('brew produced no output');
+      expect(result.stderr).not.toContain('retrying');
+    },
+    25000
+  );
+
+  it('cleans up each timed-out build before retrying, then stops at the limit', async () => {
+    const result = await run(`
+brew() { "$TEST_NODE" "$TEST_FIXTURE"; }
+sleep() { case "$1" in 5|10) return 0;; *) command sleep "$@";; esac; }
+safe_brew install php@8.4
+`);
+    expect(result.status).toBe(124);
+    expect(fs.readFileSync(path.join(root, 'attempts'), 'utf8')).toBe(
+      'attempt\nattempt\nattempt\n'
+    );
+    expect(fs.existsSync(path.join(root, 'overlap'))).toBe(false);
+    expect(pids().filter(running)).toEqual([]);
+    expect(result.stderr.match(/retrying brew command/g)).toHaveLength(2);
+    expect(result.stderr).not.toContain('attempt 4');
+  }, 25000);
+
+  it('recovers on the next attempt after cleaning up a timed-out source build', async () => {
+    const result = await run(`
+brew() {
+  if [ ! -e "$TEST_ROOT/attempts" ]; then
+    "$TEST_NODE" "$TEST_FIXTURE"
+  else
+    echo recovered
+  fi
+}
+sleep() { case "$1" in 5) return 0;; *) command sleep "$@";; esac; }
+safe_brew install php@8.4
+`);
+    expect(result.status).toBe(0);
+    expect(result.stdout).toContain('recovered\n');
+    expect(result.stderr.match(/retrying brew command/g)).toHaveLength(1);
+    expect(result.stderr).toContain('attempt 2/3, exit 124');
+    expect(pids().filter(running)).toEqual([]);
+  }, 10000);
+
+  it('retries an ordinary failure and stops after success', async () => {
+    const result = await run(`
+brew() {
+  if [ ! -e "$TEST_ROOT/failed" ]; then
+    touch "$TEST_ROOT/failed"
+    return 37
+  fi
+  printf recovered
+}
+sleep() { case "$1" in 5) return 0;; *) command sleep "$@";; esac; }
+safe_brew install php@8.4
+`);
+    expect(result.status).toBe(0);
+    expect(result.stdout).toBe('recovered');
+    expect(result.stderr.match(/retrying brew command/g)).toHaveLength(1);
+    expect(result.stderr).toContain('attempt 2/3, exit 37');
+  });
+
+  it('preserves the opt-out from the watchdog and retries', async () => {
+    const result = await run(
+      'brew() { printf disabled; return 37; }; safe_brew install php@8.4',
+      {SETUP_PHP_BREW_WATCHDOG: 'false'}
+    );
+    expect(result).toEqual({status: 37, stdout: 'disabled', stderr: ''});
+  });
+
+  it.each([0, 37])('preserves output and exit status %s', async status => {
+    const result = await run(
+      `run_with_inactivity_watchdog bash -c 'printf out; printf err >&2; exit ${status}'`
+    );
+    expect(result).toEqual({status, stdout: 'out', stderr: 'err'});
+  });
+
+  it('keeps the bottle timeout when no source build is running', async () => {
+    const result = await run(
+      'run_with_inactivity_watchdog "$TEST_NODE" -e "setTimeout(() => {}, 6000)"',
+      {SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: '4'}
+    );
+    expect(result.status).toBe(124);
+    expect(result.stderr).toContain('no output for 1s; terminating');
+  }, 10000);
+
+  it('ignores source builds outside the watched process tree', async () => {
+    const otherBuild = spawn(
+      process.execPath,
+      ['-e', 'setTimeout(() => {}, 10000)', buildScript],
+      {stdio: 'ignore'}
+    );
+    try {
+      const result = await run(
+        'run_with_inactivity_watchdog "$TEST_NODE" -e "setTimeout(() => {}, 6000)"',
+        {SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: '4'}
+      );
+      expect(result.status).toBe(124);
+      expect(result.stderr).toContain('no output for 1s; terminating');
+    } finally {
+      otherBuild.kill('SIGKILL');
+    }
+  }, 10000);
+
+  it.each(['', '4'])(
+    'allows quiet source builds past the bottle timeout with source timeout=%s',
+    async sourceTimeout => {
+      const result = await run(
+        'run_with_inactivity_watchdog "$TEST_NODE" "$TEST_FIXTURE"',
+        {
+          SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: sourceTimeout,
+          TEST_BUILD_DURATION: '2500'
+        }
+      );
+      expect(result.status).toBe(0);
+      expect(fs.existsSync(path.join(root, 'built'))).toBe(true);
+      expect(pids().filter(running)).toEqual([]);
+      expect(result.stderr).not.toContain('terminating');
+    },
+    10000
+  );
+
+  it('terminates a stalled source build at its longer timeout', async () => {
+    const result = await run(
+      'run_with_inactivity_watchdog "$TEST_NODE" "$TEST_FIXTURE"',
+      {SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: '3'}
+    );
+    expect(result.status).toBe(124);
+    expect(result.stderr).toContain('no output for 3s; terminating');
+    expect(pids().filter(running)).toEqual([]);
+  }, 10000);
+
+  it('restores the bottle timeout after the source build finishes', async () => {
+    const result = await run(
+      'run_with_inactivity_watchdog "$TEST_NODE" "$TEST_FIXTURE"',
+      {
+        SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: '4',
+        TEST_BUILD_DURATION: '2500',
+        TEST_AFTER_BUILD_DELAY: '6000'
+      }
+    );
+    expect(result.status).toBe(124);
+    expect(fs.existsSync(path.join(root, 'built'))).toBe(true);
+    expect(result.stderr).toContain('no output for 1s; terminating');
+    expect(pids().filter(running)).toEqual([]);
+  }, 10000);
+
+  it.each(['stdout', 'stderr'] as const)(
+    'counts partial output on %s as activity',
+    async stream => {
+      const result = await run(
+        `run_with_inactivity_watchdog "$TEST_NODE" -e '
+let count = 0;
+const timer = setInterval(() => {
+  process.${stream}.write(".");
+  if (++count === 25) clearInterval(timer);
+}, 100);
+'`,
+        {SETUP_PHP_BREW_INACTIVITY_TIMEOUT: '2'}
+      );
+      expect(result.status).toBe(0);
+      expect(result[stream]).toBe('.'.repeat(25));
+      expect(result.stderr).not.toContain('terminating');
+    },
+    10000
+  );
+});

__tests__/darwin.test.ts

@@ -0,0 +1,151 @@
+import {spawnSync} from 'child_process';
+import fs from 'fs';
+import path from 'path';
+
+const darwin = fs
+  .readFileSync(path.join(__dirname, '../src/scripts/darwin.sh'), 'utf8')
+  .split('\n# Variables\n')[0];
+const describeUnix = process.platform === 'win32' ? describe.skip : describe;
+
+describeUnix('macOS PHP installation', () => {
+  const run = (env: NodeJS.ProcessEnv) =>
+    spawnSync(
+      'bash',
+      [
+        '-c',
+        `${darwin}
+uname() { echo "$TEST_ARCH"; }
+setup_cached_versions() { echo cache; return "$TEST_CACHE_STATUS"; }
+update_dependencies() { echo update; }
+add_brew_tap() { echo tap; }
+safe_brew() {
+  echo "brew $*"
+  case "$*" in
+    *--only-dependencies*) return "$TEST_DEPENDENCY_STATUS";;
+    install*) return "$TEST_INSTALL_STATUS";;
+    upgrade*) return "$TEST_UPGRADE_STATUS";;
+  esac
+}
+brew() { echo "brew $*"; }
+add_php "$TEST_ACTION" "$TEST_EXISTING_VERSION"
+`
+      ],
+      {
+        encoding: 'utf8',
+        timeout: 5000,
+        env: {
+          ...process.env,
+          TEST_ARCH: 'arm64',
+          TEST_ACTION: 'install',
+          TEST_EXISTING_VERSION: 'false',
+          TEST_CACHE_STATUS: '0',
+          TEST_DEPENDENCY_STATUS: '0',
+          TEST_INSTALL_STATUS: '0',
+          TEST_UPGRADE_STATUS: '0',
+          version: '8.4',
+          debug: 'none',
+          ts: 'nts',
+          runner: 'github',
+          use_package_cache: 'true',
+          php_tap: 'shivammathur/homebrew-php',
+          ...env
+        }
+      }
+    );
+
+  it.each([
+    ['arm64', '0', false, 0],
+    ['arm64', '37', true, 0],
+    ['x86_64', '0', false, 0],
+    ['x86_64', '37', false, 1]
+  ])(
+    'preserves the cache fallback on %s with cache status %s',
+    (arch, cacheStatus, fallback, status) => {
+      const result = run({TEST_ARCH: arch, TEST_CACHE_STATUS: cacheStatus});
+      expect(result.error).toBeUndefined();
+      expect(result.stderr).toBe('');
+      expect(result.status).toBe(status);
+      expect(result.stdout).toContain('cache\n');
+      expect(result.stdout.includes('brew install')).toBe(fallback);
+    }
+  );
+
+  it.each([
+    ['install', '124', '0', '0', 124, 1],
+    ['install', '37', '0', '0', 37, 1],
+    ['install', '0', '124', '0', 124, 2],
+    ['install', '0', '37', '124', 124, 3],
+    ['install', '0', '37', '37', 37, 3],
+    ['upgrade', '124', '0', '0', 124, 1],
+    ['upgrade', '0', '0', '124', 124, 2],
+    ['upgrade', '0', '0', '37', 37, 2]
+  ])(
+    'stops %s after failures (dependencies=%s, install=%s, upgrade=%s)',
+    (action, dependencyStatus, installStatus, upgradeStatus, status, calls) => {
+      const result = run({
+        TEST_ACTION: action,
+        TEST_EXISTING_VERSION: action === 'upgrade' ? '8.4.10' : 'false',
+        TEST_CACHE_STATUS: '37',
+        TEST_DEPENDENCY_STATUS: dependencyStatus,
+        TEST_INSTALL_STATUS: installStatus,
+        TEST_UPGRADE_STATUS: upgradeStatus
+      });
+      expect(result.error).toBeUndefined();
+      expect(result.stderr).toBe('');
+      expect(result.status).toBe(status);
+      expect(result.stdout.match(/^brew /gm)).toHaveLength(calls);
+      expect(result.stdout).not.toContain('brew link');
+    }
+  );
+
+  it('retains the upgrade fallback for an install failure other than a timeout', () => {
+    const result = run({TEST_CACHE_STATUS: '37', TEST_INSTALL_STATUS: '1'});
+    expect(result.error).toBeUndefined();
+    expect(result.status).toBe(0);
+    expect(result.stdout).toContain(
+      'brew upgrade -f --overwrite shivammathur/php/php@8.4\n'
+    );
+    expect(result.stdout).toContain('brew link --force --overwrite php@8.4\n');
+  });
+
+  it('reuses an existing installation without invoking the cache or Homebrew install', () => {
+    const result = run({TEST_EXISTING_VERSION: '8.4.10'});
+    expect(result.status).toBe(0);
+    expect(result.stdout).toBe(
+      'brew unlink php@8.4\nbrew link --force --overwrite php@8.4\n'
+    );
+  });
+
+  it.each([
+    ['debug', 'nts', '-debug'],
+    ['none', 'zts', '-zts'],
+    ['debug', 'zts', '-debug-zts']
+  ])('keeps cache fallback for debug=%s, ts=%s', (debug, ts, suffix) => {
+    const result = run({
+      TEST_EXISTING_VERSION: '8.4.10',
+      TEST_CACHE_STATUS: '37',
+      debug,
+      ts
+    });
+    expect(result.status).toBe(0);
+    expect(result.stdout).toContain('cache\n');
+    expect(result.stdout).toContain(
+      `brew install --skip-link -f --overwrite shivammathur/php/php@8.4${suffix}\n`
+    );
+  });
+
+  it.each([
+    ['self-hosted', 'true'],
+    ['github', 'false']
+  ])('uses Homebrew for runner=%s, cache=%s', (runner, use_package_cache) => {
+    const result = run({runner, use_package_cache, TEST_CACHE_STATUS: '37'});
+    expect(result.error).toBeUndefined();
+    expect(result.stderr).toBe('');
+    expect(result.status).toBe(0);
+    expect(result.stdout).not.toContain('cache\n');
+    expect(result.stdout).toContain(
+      'brew install --skip-link -f --overwrite shivammathur/php/php@8.4\n'
+    );
+    expect(result.stdout).toContain('brew link --force --overwrite php@8.4\n');
+  });
+});

src/scripts/darwin.sh

@@ -183,6 +183,7 @@ setup_cached_versions() {
 
 # Function to setup PHP 5.6 and newer using Homebrew.
 add_php() {
+  local exit_code
   action=$1
   existing_version=$2
   suffix="$(get_php_formula_suffix)"
@@ -198,14 +199,18 @@ add_php() {
   fi
   if [[ "$existing_version" != "false" && -z "$suffix" ]]; then
     if [ "$action" = "upgrade" ]; then
-      safe_brew install --only-dependencies "$php_formula"
+      safe_brew install --only-dependencies "$php_formula" || return $?
-      safe_brew upgrade -f --overwrite "$php_formula"
+      safe_brew upgrade -f --overwrite "$php_formula" || return $?
     else
       brew unlink "$php_keg"
     fi
   else
-    safe_brew install --only-dependencies "$php_formula"
+    safe_brew install --only-dependencies "$php_formula" || return $?
-    safe_brew install --skip-link -f --overwrite "$php_formula" 2>/dev/null || safe_brew upgrade -f --overwrite "$php_formula"
+    safe_brew install --skip-link -f --overwrite "$php_formula" 2>/dev/null || {
+      exit_code=$?
+      [ "$exit_code" -ne 124 ] || return "$exit_code"
+      safe_brew upgrade -f --overwrite "$php_formula" || return $?
+    }
   fi
   brew link --force --overwrite "$php_keg" || (sudo chown -R "$(id -un)":"$(id -gn)" "$brew_prefix" && brew link --force --overwrite "$php_keg")
 }

src/scripts/tools/brew.sh

@@ -55,25 +55,45 @@ get_file_mtime() {
   fi
 }
 
-# Function to terminate a process and its direct children.
+# Function to list descendants before their parents, including separate sessions.
-terminate_process_tree() {
+get_process_tree() {
   local pid=$1
   local children child
   children=$(pgrep -P "$pid" 2>/dev/null || true)
-  kill -TERM "$pid" >/dev/null 2>&1 || true
   for child in $children; do
-    terminate_process_tree "$child"
+    get_process_tree "$child"
+  done
+  echo "$pid"
+}
+
+# Function to detect Homebrew's source-build worker, even with buffered output.
+is_brew_building_from_source() {
+  local pid
+  for pid in $(get_process_tree "$1"); do
+    if ps -ww -p "$pid" -o command= 2>/dev/null | grep -qE '/Homebrew/build[.]rb([[:space:]]|$)'; then
+      return 0
+    fi
+  done
+  return 1
+}
+
+# Function to terminate the entire tree captured before any parents can exit.
+terminate_process_tree() {
+  local pids pid
+  pids=$(get_process_tree "$1")
+  for pid in $pids; do
+    kill -TERM "$pid" >/dev/null 2>&1 || true
   done
   sleep 2
-  kill -KILL "$pid" >/dev/null 2>&1 || true
+  for pid in $pids; do
-  for child in $children; do
+    kill -KILL "$pid" >/dev/null 2>&1 || true
-    terminate_process_tree "$child"
   done
 }
 
 # Function to run a command with an inactivity watchdog.
 run_with_inactivity_watchdog() {
   local timeout_secs="${SETUP_PHP_BREW_INACTIVITY_TIMEOUT:-180}"
+  local source_timeout_secs="${SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT:-1800}"
   local poll_secs="${SETUP_PHP_BREW_WATCHDOG_POLL:-5}"
   local tmp_dir stdout_fifo stderr_fifo stdout_log stderr_log timeout_file
   local command_pid stdout_reader_pid stderr_reader_pid monitor_pid exit_code
@@ -93,36 +113,39 @@ run_with_inactivity_watchdog() {
   ("$@" >"$stdout_fifo" 2>"$stderr_fifo") &
   command_pid=$!
 
-  (
+  tee "$stdout_log" <"$stdout_fifo" &
-    while IFS= read -r line || [ -n "$line" ]; do
-      printf '%s\n' "$line"
-      printf '%s\n' "$line" >>"$stdout_log"
-    done <"$stdout_fifo"
-  ) &
   stdout_reader_pid=$!
 
-  (
+  tee "$stderr_log" <"$stderr_fifo" >&2 &
-    while IFS= read -r line || [ -n "$line" ]; do
-      printf '%s\n' "$line" >&2
-      printf '%s\n' "$line" >>"$stderr_log"
-    done <"$stderr_fifo"
-  ) &
   stderr_reader_pid=$!
 
   (
     local last_activity current_activity current_err_activity now
+    local building_from_source=false was_building_from_source=false active_timeout_secs
     last_activity=$(get_file_mtime "$stdout_log")
     current_err_activity=$(get_file_mtime "$stderr_log")
     [ "$current_err_activity" -gt "$last_activity" ] && last_activity="$current_err_activity"
     while kill -0 "$command_pid" >/dev/null 2>&1; do
       sleep "$poll_secs"
+      kill -0 "$command_pid" >/dev/null 2>&1 || break
+      now=$(date +%s)
+      active_timeout_secs="$timeout_secs"
+      building_from_source=false
+      if is_brew_building_from_source "$command_pid"; then
+        building_from_source=true
+        active_timeout_secs="$source_timeout_secs"
+      fi
+      if [ "$building_from_source" != "$was_building_from_source" ]; then
+        last_activity="$now"
+        was_building_from_source="$building_from_source"
+      fi
       current_activity=$(get_file_mtime "$stdout_log")
       [ "$current_activity" -gt "$last_activity" ] && last_activity="$current_activity"
       current_err_activity=$(get_file_mtime "$stderr_log")
       [ "$current_err_activity" -gt "$last_activity" ] && last_activity="$current_err_activity"
       now=$(date +%s)
-      if [ $((now - last_activity)) -ge "$timeout_secs" ]; then
+      if [ $((now - last_activity)) -ge "$active_timeout_secs" ]; then
-        printf "\nsetup-php: brew produced no output for %ss; terminating and retrying...\n" "$timeout_secs" >&2
+        printf "\nsetup-php: brew produced no output for %ss; terminating...\n" "$active_timeout_secs" >&2
         : >"$timeout_file"
         terminate_process_tree "$command_pid"
         break
@@ -131,12 +154,15 @@ run_with_inactivity_watchdog() {
   ) &
   monitor_pid=$!
 
-  wait "$command_pid"
+  exit_code=0
-  exit_code=$?
+  wait "$command_pid" || exit_code=$?
+  # Let timeout cleanup finish killing source-build descendants before retrying.
+  if [ ! -e "$timeout_file" ]; then
+    kill "$monitor_pid" >/dev/null 2>&1 || true
+  fi
+  wait "$monitor_pid" 2>/dev/null || true
   wait "$stdout_reader_pid" 2>/dev/null || true
   wait "$stderr_reader_pid" 2>/dev/null || true
-  kill "$monitor_pid" >/dev/null 2>&1 || true
-  wait "$monitor_pid" 2>/dev/null || true
 
   if [ -e "$timeout_file" ]; then
     rm -rf "$tmp_dir"