From ca9544522ebc3e622b332a4cd9a3d160931814d6 Mon Sep 17 00:00:00 2001
From: Alexander Larsson <alexl@redhat.com>
Date: Mon, 16 Jun 2014 15:30:42 +0200
Subject: [PATCH] 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      |  8 +++++++-
 namespaces/init.go      | 10 +++++++---
 namespaces/sync_pipe.go | 31 ++++++++++++++++++++++++++++++-
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/namespaces/exec.go b/namespaces/exec.go
index c4b724a7..e32da4b8 100644
--- a/namespaces/exec.go
+++ b/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()
diff --git a/namespaces/init.go b/namespaces/init.go
index 6c474514..4a6ce6e3 100644
--- a/namespaces/init.go
+++ b/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 {
diff --git a/namespaces/sync_pipe.go b/namespaces/sync_pipe.go
index 57851c5c..ae6fefc3 100644
--- a/namespaces/sync_pipe.go
+++ b/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
+	}
+}