Merge pull request #44215 from corhere/fix-unlockosthread-pdeathsig

Stop subprocesses from getting unexpectedly killed

daemon/graphdriver/overlay2/mount.go

@@ -50,6 +50,13 @@ func mountFrom(dir, device, target, mType string, flags uintptr, label string) e
 	output := bytes.NewBuffer(nil)
 	cmd.Stdout = output
 	cmd.Stderr = output
+
+	// reexec.Command() sets cmd.SysProcAttr.Pdeathsig on Linux, which
+	// causes the started process to be signaled when the creating OS thread
+	// dies. Ensure that the reexec is not prematurely signaled. See
+	// https://go.dev/issue/27505 for more information.
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	if err := cmd.Start(); err != nil {
 		w.Close()
 		return fmt.Errorf("mountfrom error on re-exec cmd: %v", err)

daemon/logger/journald/read.go

@@ -404,6 +404,7 @@ func waitUntilFlushedImpl(s *journald) error {
 	go func() {
 		defer close(flushed)
 		runtime.LockOSThread()
+		defer runtime.UnlockOSThread()
 
 		var (
 			j   *sdjournal.Journal

libcontainerd/supervisor/remote_daemon.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strconv"
 	"strings"
 	"time"
@@ -200,18 +201,34 @@ func (r *remote) startContainerd() error {
 			cmd.Env = append(cmd.Env, e)
 		}
 	}
-	if err := cmd.Start(); err != nil {
+
-		return err
+	startedCh := make(chan error)
+	go func() {
+		// On Linux, when cmd.SysProcAttr.Pdeathsig is set,
+		// the signal is sent to the subprocess when the creating thread
+		// terminates. The runtime terminates a thread if a goroutine
+		// exits while locked to it. Prevent the containerd process
+		// from getting killed prematurely by ensuring that the thread
+		// used to start it remains alive until it or the daemon process
+		// exits. See https://go.dev/issue/27505 for more details.
+		runtime.LockOSThread()
+		defer runtime.UnlockOSThread()
+		err := cmd.Start()
+		startedCh <- err
+		if err != nil {
+			return
 		}
 
 		r.daemonWaitCh = make(chan struct{})
-	go func() {
 		// Reap our child when needed
 		if err := cmd.Wait(); err != nil {
 			r.logger.WithError(err).Errorf("containerd did not exit successfully")
 		}
 		close(r.daemonWaitCh)
 	}()
+	if err := <-startedCh; err != nil {
+		return err
+	}
 
 	r.daemonPid = cmd.Process.Pid
 

libnetwork/portmapper/proxy_linux.go

@@ -6,6 +6,7 @@ import (
 	"net"
 	"os"
 	"os/exec"
+	"runtime"
 	"strconv"
 	"syscall"
 	"time"
@@ -37,9 +38,10 @@ func newProxyCommand(proto string, hostIP net.IP, hostPort int, containerIP net.
 			Path: path,
 			Args: args,
 			SysProcAttr: &syscall.SysProcAttr{
-				Pdeathsig: syscall.SIGTERM, // send a sigterm to the proxy if the daemon process dies
+				Pdeathsig: syscall.SIGTERM, // send a sigterm to the proxy if the creating thread in the daemon process dies (https://go.dev/issue/27505)
 			},
 		},
+		wait: make(chan error, 1),
 	}, nil
 }
 
@@ -47,6 +49,7 @@ func newProxyCommand(proto string, hostIP net.IP, hostPort int, containerIP net.
 // proxies as separate processes.
 type proxyCommand struct {
 	cmd  *exec.Cmd
+	wait chan error
 }
 
 func (p *proxyCommand) Start() error {
@@ -56,7 +59,29 @@ func (p *proxyCommand) Start() error {
 	}
 	defer r.Close()
 	p.cmd.ExtraFiles = []*os.File{w}
-	if err := p.cmd.Start(); err != nil {
+
+	// As p.cmd.SysProcAttr.Pdeathsig is set, the signal will be sent to the
+	// process when the OS thread on which p.cmd.Start() was executed dies.
+	// If the thread is allowed to be released back into the goroutine
+	// thread pool, the thread could get terminated at any time if a
+	// goroutine gets scheduled onto it which calls runtime.LockOSThread()
+	// and exits without a matching number of runtime.UnlockOSThread()
+	// calls. Ensure that the thread from which Start() is called stays
+	// alive until the proxy or the daemon process exits to prevent the
+	// proxy from getting terminated early. See https://go.dev/issue/27505
+	// for more details.
+	started := make(chan error)
+	go func() {
+		runtime.LockOSThread()
+		defer runtime.UnlockOSThread()
+		err := p.cmd.Start()
+		started <- err
+		if err != nil {
+			return
+		}
+		p.wait <- p.cmd.Wait()
+	}()
+	if err := <-started; err != nil {
 		return err
 	}
 	w.Close()
@@ -92,7 +117,7 @@ func (p *proxyCommand) Stop() error {
 		if err := p.cmd.Process.Signal(os.Interrupt); err != nil {
 			return err
 		}
-		return p.cmd.Wait()
+		return <-p.wait
 	}
 	return nil
 }

pkg/chrootarchive/archive_unix.go

@@ -95,6 +95,12 @@ func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.T
 	cmd.Stdout = output
 	cmd.Stderr = output
 
+	// reexec.Command() sets cmd.SysProcAttr.Pdeathsig on Linux, which
+	// causes the started process to be signaled when the creating OS thread
+	// dies. Ensure that the reexec is not prematurely signaled. See
+	// https://go.dev/issue/27505 for more information.
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	if err := cmd.Start(); err != nil {
 		w.Close()
 		return fmt.Errorf("Untar error on re-exec cmd: %v", err)
@@ -188,15 +194,27 @@ func invokePack(srcPath string, options *archive.TarOptions, root string) (io.Re
 		return nil, errors.Wrap(err, "error getting options pipe for tar process")
 	}
 
-	if err := cmd.Start(); err != nil {
+	started := make(chan error)
-		return nil, errors.Wrap(err, "tar error on re-exec cmd")
-	}
-
 	go func() {
+		// reexec.Command() sets cmd.SysProcAttr.Pdeathsig on Linux,
+		// which causes the started process to be signaled when the
+		// creating OS thread dies. Ensure that the subprocess is not
+		// prematurely signaled. See https://go.dev/issue/27505 for more
+		// information.
+		runtime.LockOSThread()
+		defer runtime.UnlockOSThread()
+		if err := cmd.Start(); err != nil {
+			started <- err
+			return
+		}
+		close(started)
 		err := cmd.Wait()
 		err = errors.Wrapf(err, "error processing tar file: %s", errBuff)
 		tarW.CloseWithError(err)
 	}()
+	if err := <-started; err != nil {
+		return nil, errors.Wrap(err, "tar error on re-exec cmd")
+	}
 
 	if err := json.NewEncoder(stdin).Encode(options); err != nil {
 		stdin.Close()

pkg/chrootarchive/diff_unix.go

@@ -115,6 +115,12 @@ func applyLayerHandler(dest string, layer io.Reader, options *archive.TarOptions
 	outBuf, errBuf := new(bytes.Buffer), new(bytes.Buffer)
 	cmd.Stdout, cmd.Stderr = outBuf, errBuf
 
+	// reexec.Command() sets cmd.SysProcAttr.Pdeathsig on Linux, which
+	// causes the started process to be signaled when the creating OS thread
+	// dies. Ensure that the reexec is not prematurely signaled. See
+	// https://go.dev/issue/27505 for more information.
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	if err = cmd.Run(); err != nil {
 		return 0, fmt.Errorf("ApplyLayer %s stdout: %s stderr: %s", err, outBuf, errBuf)
 	}

pkg/reexec/command_linux.go

@@ -17,6 +17,11 @@ func Self() string {
 // SysProcAttr.Pdeathsig to SIGTERM.
 // This will use the in-memory version (/proc/self/exe) of the current binary,
 // it is thus safe to delete or replace the on-disk binary (os.Args[0]).
+//
+// As SysProcAttr.Pdeathsig is set, the signal will be sent to the process when
+// the OS thread which created the process dies. It is the caller's
+// responsibility to ensure that the creating thread is not terminated
+// prematurely. See https://go.dev/issue/27505 for more details.
 func Command(args ...string) *exec.Cmd {
 	return &exec.Cmd{
 		Path: Self(),