Merge pull request #6307 from nalind/reap

Run: reap stray processes

Makefile

@@ -59,7 +59,7 @@ export GOLANGCI_LINT_VERSION := 2.1.0
 #     Note: Uses the -N -l go compiler options to disable compiler optimizations
 #           and inlining. Using these build options allows you to subsequently
 #           use source debugging tools like delve.
-all: bin/buildah bin/imgtype bin/copy bin/inet bin/tutorial bin/dumpspec bin/passwd docs
+all: bin/buildah bin/imgtype bin/copy bin/inet bin/tutorial bin/dumpspec bin/passwd bin/crash bin/wait docs
 
 bin/buildah: $(SOURCES) internal/mkcw/embed/entrypoint_amd64.gz
 	$(GO_BUILD) $(BUILDAH_LDFLAGS) $(GO_GCFLAGS) "$(GOGCFLAGS)" -o $@ $(BUILDFLAGS) ./cmd/buildah
@@ -91,6 +91,12 @@ bin/buildah.%: $(SOURCES)
 	mkdir -p ./bin
 	GOOS=$(word 2,$(subst ., ,$@)) GOARCH=$(word 3,$(subst ., ,$@)) $(GO_BUILD) $(BUILDAH_LDFLAGS) -o $@ -tags "containers_image_openpgp" ./cmd/buildah
 
+bin/crash: $(SOURCES)
+	$(GO_BUILD) $(BUILDAH_LDFLAGS) -o $@ $(BUILDFLAGS) ./tests/crash
+
+bin/wait: $(SOURCES)
+	$(GO_BUILD) $(BUILDAH_LDFLAGS) -o $@ $(BUILDFLAGS) ./tests/wait
+
 bin/dumpspec: $(SOURCES)
 	$(GO_BUILD) $(BUILDAH_LDFLAGS) -o $@ $(BUILDFLAGS) ./tests/dumpspec
 

rpm/buildah.spec

@@ -139,6 +139,8 @@ export BUILDTAGS+=" libtrust_openssl"
 %gobuild -o bin/inet ./tests/inet
 %gobuild -o bin/dumpspec ./tests/dumpspec
 %gobuild -o bin/passwd ./tests/passwd
+%gobuild -o bin/crash ./tests/crash
+%gobuild -o bin/wait ./tests/wait
 %{__make} docs
 
 %install
@@ -152,6 +154,8 @@ cp bin/tutorial %{buildroot}/%{_bindir}/%{name}-tutorial
 cp bin/inet     %{buildroot}/%{_bindir}/%{name}-inet
 cp bin/dumpspec %{buildroot}/%{_bindir}/%{name}-dumpspec
 cp bin/passwd %{buildroot}/%{_bindir}/%{name}-passwd
+cp bin/crash %{buildroot}/%{_bindir}/%{name}-crash
+cp bin/wait %{buildroot}/%{_bindir}/%{name}-wait
 
 rm %{buildroot}%{_datadir}/%{name}/test/system/tools/build/*
 
@@ -178,6 +182,8 @@ rm %{buildroot}%{_datadir}/%{name}/test/system/tools/build/*
 %{_bindir}/%{name}-inet
 %{_bindir}/%{name}-dumpspec
 %{_bindir}/%{name}-passwd
+%{_bindir}/%{name}-crash
+%{_bindir}/%{name}-wait
 %{_datadir}/%{name}/test
 
 %changelog

run_common.go

@@ -452,6 +452,7 @@ func runUsingRuntime(options RunOptions, configureNetwork bool, moreCreateArgs [
 
 	// Lock the caller to a single OS-level thread.
 	runtime.LockOSThread()
+	defer reapStrays()
 
 	// Set up bind mounts for things that a namespaced user might not be able to get to directly.
 	unmountAll, err := bind.SetupIntermediateMountNamespace(spec, bundlePath)
@@ -1081,6 +1082,23 @@ func runAcceptTerminal(logger *logrus.Logger, consoleListener *net.UnixListener,
 	return terminalFD, nil
 }
 
+func reapStrays() {
+	// Reap the exit status of anything that was reparented to us, not that
+	// we care about their exit status.
+	logrus.Debugf("checking for reparented child processes")
+	for range 100 {
+		wpid, err := unix.Wait4(-1, nil, unix.WNOHANG, nil)
+		if err != nil {
+			break
+		}
+		if wpid == 0 {
+			time.Sleep(100 * time.Millisecond)
+		} else {
+			logrus.Debugf("caught reparented child process %d", wpid)
+		}
+	}
+}
+
 func runUsingRuntimeMain() {
 	var options runUsingRuntimeSubprocOptions
 	// Set logging.
@@ -1129,6 +1147,7 @@ func runUsingRuntimeMain() {
 
 	// Run the container, start to finish.
 	status, err := runUsingRuntime(options.Options, options.ConfigureNetwork, options.MoreCreateArgs, ospec, options.BundlePath, options.ContainerName, containerCreateW, containerStartR)
+	reapStrays()
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "error running container: %v\n", err)
 		os.Exit(1)

tests/crash/crash_notunix.go

@@ -0,0 +1,9 @@
+//go:build !unix
+
+package main
+
+// This is really only here to prevent complaints about the source directory
+// for a helper that's used in a Unix-specific test not having something that
+// will compile on non-Unix platforms.
+func main() {
+}

tests/crash/crash_unix.go

@@ -0,0 +1,40 @@
+package main
+
+import (
+	"os"
+	"os/exec"
+	"syscall"
+
+	"github.com/sirupsen/logrus"
+)
+
+// Launch a child process and behave, rather unconvincingly, as an OCI runtime.
+func main() {
+	if err := exec.Command("sh", "-c", "sleep 0 &").Start(); err != nil {
+		logrus.Fatalf("%v", err)
+	}
+	for _, arg := range os.Args {
+		switch arg {
+		case "create":
+			logrus.Info("created\n")
+			os.Exit(0)
+		case "delete":
+			logrus.Info("deleted\n")
+			os.Exit(0)
+		case "kill":
+			logrus.Info("killed\n")
+			os.Exit(0)
+		case "start":
+			logrus.Info("starting\n")
+			// crash here, so that our caller, being run under
+			// "wait", will have to reap us and our errant child
+			// process, lest "wait" complain
+			if err := syscall.Kill(os.Getpid(), syscall.SIGSEGV); err != nil {
+				logrus.Fatalf("awkward: error sending SIGSEGV to myself: %v", err)
+			}
+		}
+	}
+	if err := syscall.Kill(os.Getpid(), syscall.SIGSEGV); err != nil {
+		logrus.Fatalf("awkward: error sending SIGSEGV to myself: %v", err)
+	}
+}

tests/helpers.bash

@@ -9,6 +9,8 @@ COPY_BINARY=${COPY_BINARY:-$TEST_SOURCES/../bin/copy}
 TUTORIAL_BINARY=${TUTORIAL_BINARY:-$TEST_SOURCES/../bin/tutorial}
 INET_BINARY=${INET_BINARY:-$TEST_SOURCES/../bin/inet}
 DUMPSPEC_BINARY=${DUMPSPEC_BINARY:-$TEST_SOURCES/../bin/dumpspec}
+CRASH_BINARY=${CRASH_BINARY:-$TEST_SOURCES/../bin/crash}
+WAIT_BINARY=${WAIT_BINARY:-$TEST_SOURCES/../bin/wait}
 PASSWD_BINARY=${PASSWD_BINARY:-$TEST_SOURCES/../bin/passwd}
 STORAGE_DRIVER=${STORAGE_DRIVER:-vfs}
 PATH=$(dirname ${BASH_SOURCE})/../bin:${PATH}

tests/run.bats

@@ -1045,3 +1045,16 @@ _EOF
   cat ${TEST_SCRATCH_DIR}/mountinfo3
   assert $(cat ${TEST_SCRATCH_DIR}/mountinfo3) -eq 1
 }
+
+@test "run reaps stray processes" {
+  if test `uname` != Linux ; then
+    skip "not meaningful except on Linux"
+  fi
+  _prefetch busybox
+  run_buildah from --pull=never --quiet busybox
+  local cid="$output"
+  echo '$' ${WAIT_BINARY} ${BUILDAH_BINARY} ${BUILDAH_REGISTRY_OPTS} ${ROOTDIR_OPTS} run --runtime=${CRASH_BINARY} "$cid" pwd
+  run ${WAIT_BINARY} ${BUILDAH_BINARY} ${BUILDAH_REGISTRY_OPTS} ${ROOTDIR_OPTS} run --runtime=${CRASH_BINARY} "$cid" pwd
+  echo "$output"
+  assert "$output" !~ "caught reparented child process"
+}

tests/wait/wait_notunix.go

@@ -0,0 +1,9 @@
+//go:build !unix
+
+package main
+
+// This is really only here to prevent complaints about the source directory
+// for a helper that's used in a Unix-specific test not having something that
+// will compile on non-Unix platforms.
+func main() {
+}

tests/wait/wait_unix.go

@@ -0,0 +1,51 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"time"
+
+	"github.com/sirupsen/logrus"
+	"golang.org/x/sys/unix"
+)
+
+func main() {
+	if err := unix.Prctl(unix.PR_SET_CHILD_SUBREAPER, uintptr(1), 0, 0, 0); err != nil {
+		fmt.Fprintf(os.Stderr, "%v", err)
+		os.Exit(1)
+	}
+	if len(os.Args) < 2 {
+		fmt.Fprintf(os.Stderr, "usage: %s [CMD ...]\n", filepath.Base(os.Args[0]))
+		os.Exit(1)
+	}
+	cmd := exec.Command(os.Args[1], os.Args[2:]...)
+	cmd.Stdin = os.Stdin
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+	err := cmd.Run()
+	caught := false
+	for range 100 {
+		wpid, err := unix.Wait4(-1, nil, unix.WNOHANG, nil)
+		if err != nil {
+			break
+		}
+		if wpid == 0 {
+			time.Sleep(100 * time.Millisecond)
+		} else {
+			// log an error: the child process was expected to reap
+			// its own reparented child processes; we shouldn't
+			// have had to clean them up on its behalf
+			logrus.Errorf("caught reparented child process %d", wpid)
+			caught = true
+		}
+	}
+	if !caught {
+		if err == nil {
+			return
+		}
+		fmt.Fprintf(os.Stderr, "%v", err)
+	}
+	os.Exit(1)
+}