Report child error better (and later)

We use a unix domain socketpair instead of a pipe for the sync pipe,
which allows us to use two-way shutdown. After sending the
context we shut down the write side which lets the child know
it finished reading.

We then block on a read in the parent for the child closing the file
(ensuring we close our version of it too) to sync for when the child
is finished initializing. If the read is non-empty we assume this
is an error report and fail with an error. Otherwise we continue as
before.

This also means we're now calling back the start callback later,
meaning at that point its more likely to have succeeded, as well as
having consumed all the container resources (like volume mounts,
making it safe to e.g. unmount them when the start callback is
called).

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)

namespaces/exec.go

@@ -32,6 +32,7 @@ func Exec(container *libcontainer.Config, term Terminal, rootfs, dataPath string
 	if err != nil {
 		return -1, err
 	}
+	defer syncPipe.Close()
 
 	if container.Tty {
 		master, console, err = system.CreateMasterAndConsole()
@@ -52,6 +53,9 @@ func Exec(container *libcontainer.Config, term Terminal, rootfs, dataPath string
 		return -1, err
 	}
 
+	// Now we passed the pipe to the child, close our side
+	syncPipe.CloseChild()
+
 	started, err := system.GetProcessStartTime(command.Process.Pid)
 	if err != nil {
 		return -1, err
@@ -90,7 +94,9 @@ func Exec(container *libcontainer.Config, term Terminal, rootfs, dataPath string
 	defer libcontainer.DeleteState(dataPath)
 
 	// Sync with child
-	syncPipe.Close()
+	if err := syncPipe.BlockOnChild(); err != nil {
+		return -1, err
+	}
 
 	if startCallback != nil {
 		startCallback()

namespaces/init.go

@@ -27,7 +27,13 @@ import (
 // Move this to libcontainer package.
 // Init is the init process that first runs inside a new namespace to setup mounts, users, networking,
 // and other options required for the new container.
-func Init(container *libcontainer.Config, uncleanRootfs, consolePath string, syncPipe *SyncPipe, args []string) error {
+func Init(container *libcontainer.Config, uncleanRootfs, consolePath string, syncPipe *SyncPipe, args []string) (err error) {
+	defer func() {
+		if err != nil {
+			syncPipe.ReportError(err)
+		}
+	}()
+
 	rootfs, err := utils.ResolveRootfs(uncleanRootfs)
 	if err != nil {
 		return err
@@ -42,10 +48,8 @@ func Init(container *libcontainer.Config, uncleanRootfs, consolePath string, syn
 	// We always read this as it is a way to sync with the parent as well
 	networkState, err := syncPipe.ReadFromParent()
 	if err != nil {
-		syncPipe.Close()
 		return err
 	}
-	syncPipe.Close()
 
 	if consolePath != "" {
 		if err := console.OpenAndDup(consolePath); err != nil {

namespaces/sync_pipe.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"syscall"
 
 	"github.com/docker/libcontainer/network"
 )
@@ -18,10 +19,14 @@ type SyncPipe struct {
 
 func NewSyncPipe() (s *SyncPipe, err error) {
 	s = &SyncPipe{}
-	s.child, s.parent, err = os.Pipe()
+
+	fds, err := syscall.Socketpair(syscall.AF_LOCAL, syscall.SOCK_STREAM|syscall.SOCK_CLOEXEC, 0)
 	if err != nil {
 		return nil, err
 	}
+	s.child = os.NewFile(uintptr(fds[0]), "child syncpipe")
+	s.parent = os.NewFile(uintptr(fds[1]), "parent syncpipe")
+
 	return s, nil
 }
 
@@ -51,6 +56,18 @@ func (s *SyncPipe) SendToChild(networkState *network.NetworkState) error {
 		return err
 	}
 	s.parent.Write(data)
+	syscall.Shutdown(int(s.parent.Fd()), syscall.SHUT_WR)
+	return nil
+}
+
+func (s *SyncPipe) BlockOnChild() error {
+	data, err := ioutil.ReadAll(s.parent)
+	if err != nil {
+		return nil
+	}
+	if len(data) > 0 {
+		return fmt.Errorf("Child error: %s", string(data))
+	}
 	return nil
 }
 
@@ -69,6 +86,11 @@ func (s *SyncPipe) ReadFromParent() (*network.NetworkState, error) {
 
 }
 
+func (s *SyncPipe) ReportError(err error) {
+	s.child.Write([]byte(err.Error()))
+	s.CloseChild()
+}
+
 func (s *SyncPipe) Close() error {
 	if s.parent != nil {
 		s.parent.Close()
@@ -78,3 +100,10 @@ func (s *SyncPipe) Close() error {
 	}
 	return nil
 }
+
+func (s *SyncPipe) CloseChild() {
+	if s.child != nil {
+		s.child.Close()
+		s.child = nil
+	}
+}