Skip to content

Commit e37aebb

Browse files
committed
buildctl - fix job reaping race that hangs parallel builds under bash 5.3
1 parent 9970a9b commit e37aebb

1 file changed

Lines changed: 33 additions & 11 deletions

File tree

build/buildctl

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -226,13 +226,19 @@ start_built_listener() {
226226
fi
227227
export BUILDCTL_PARALLEL=1
228228
logmsg "-- Background built package thread started..."
229+
# The pipe is held open read-write for the lifetime of the listener so
230+
# that a reader always exists. Otherwise there is a window, while the
231+
# listener is processing a line, in which a writer can open the pipe
232+
# and then be killed by SIGPIPE when the read side is closed before
233+
# the write lands; the writing job then exits with status 141 and
234+
# loses any lines it had still to write.
229235
while :; do
230-
if read line <$built_pipe; then
236+
if read -u $built_fd line; then
231237
[ $line = "quit" ] && break
232238
already_built+=([$line]=1)
233239
echo $line >> "$BUILT_CACHE"
234240
fi
235-
done &
241+
done {built_fd}<>"$built_pipe" &
236242
}
237243

238244
stop_built_listener() {
@@ -529,6 +535,16 @@ wait_for_slot() {
529535
while :; do
530536
wait -fn -p pid ${slots[*]}
531537
stat=$?
538+
539+
# If a pid was returned then a job has been reaped and its slot
540+
# must always be cleared before calling `wait` again, regardless
541+
# of the exit status.
542+
if [ -n "$pid" ]; then
543+
slot=${slotpid[$pid]}
544+
[ -n "$slot" ] && [ "${slots[$slot]}" = "$pid" ] && break
545+
logerr "wait returned pid $pid ($stat) which matches no slot"
546+
fi
547+
532548
case $stat in
533549
169) continue ;; # SIGINFO
534550
141) continue ;; # SIGPIPE
@@ -537,12 +553,9 @@ wait_for_slot() {
537553
clear_slots $stat
538554
nextslot=0
539555
return ;;
556+
*) logmsg -e "-- wait returned status $stat with no pid"
557+
continue ;;
540558
esac
541-
542-
[ -n "$pid" ] || continue
543-
slot=${slotpid[$pid]}
544-
[ -n "$slot" ] && break
545-
logmsg -n "Unknown pid $pid terminated $stat"
546559
done
547560

548561
if reap_slot $slot $stat; then
@@ -574,21 +587,30 @@ wait_for_jobs() {
574587

575588
typeset slot=
576589
typeset -i stat=0
590+
typeset pid=
577591

578592
while (( ${#slots[*]} > 0 )); do
579593
wait -fn -p pid ${slots[*]}
580594
stat=$?
595+
# As in wait_for_slot(), a returned pid means that a job has been
596+
# reaped and its slot must always be cleared, regardless of the
597+
# exit status.
598+
if [ -n "$pid" ]; then
599+
slot=${slotpid[$pid]}
600+
[ -n "$slot" ] && [ "${slots[$slot]}" = "$pid" ] || \
601+
logerr "wait returned pid $pid ($stat) which matches no slot"
602+
reap_slot $slot $stat
603+
job_status "($msg) "
604+
continue
605+
fi
581606
(( stat == 169 )) && continue # SIGINFO
582607
(( stat == 141 )) && continue # SIGPIPE
583608
(( stat == 144 )) && continue # SIGUSR1
584609
if (( stat == 127 )); then # No jobs left
585610
clear_slots $stat
586611
break
587612
fi
588-
[ -n "$pid" ] || continue
589-
slot=${slotpid[$pid]}
590-
[ -n "$slot" ] && reap_slot $slot $stat
591-
job_status "($msg) "
613+
logmsg -e "-- wait returned status $stat with no pid"
592614
done
593615
}
594616

0 commit comments

Comments
 (0)