diff --git a/Makefile b/Makefile index fd3d24ef6..d7efd9992 100644 --- a/Makefile +++ b/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 diff --git a/rpm/buildah.spec b/rpm/buildah.spec index 1d3350ffa..53db62ac5 100644 --- a/rpm/buildah.spec +++ b/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 diff --git a/run_common.go b/run_common.go index 05835028f..89e3ccac5 100644 --- a/run_common.go +++ b/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) diff --git a/tests/crash/crash_notunix.go b/tests/crash/crash_notunix.go new file mode 100644 index 000000000..4c66e9d4c --- /dev/null +++ b/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() { +} diff --git a/tests/crash/crash_unix.go b/tests/crash/crash_unix.go new file mode 100644 index 000000000..e3f135d6a --- /dev/null +++ b/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) + } +} diff --git a/tests/helpers.bash b/tests/helpers.bash index 0e80d5262..c2c287a86 100644 --- a/tests/helpers.bash +++ b/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} diff --git a/tests/run.bats b/tests/run.bats index 436d2112d..72a2f5501 100644 --- a/tests/run.bats +++ b/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" +} diff --git a/tests/wait/wait_notunix.go b/tests/wait/wait_notunix.go new file mode 100644 index 000000000..4c66e9d4c --- /dev/null +++ b/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() { +} diff --git a/tests/wait/wait_unix.go b/tests/wait/wait_unix.go new file mode 100644 index 000000000..aa54947b4 --- /dev/null +++ b/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) +}