Add integration test for unix sock cleanup

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

api/server/server.go

@@ -1580,10 +1580,13 @@ func ServeApi(job *engine.Job) engine.Status {
 			}
 			job.Eng.OnShutdown(func() {
 				if err := srv.Close(); err != nil {
-					log.Errorf("%s", err.Error())
+					log.Error(err)
 				}
 			})
-			chErrors <- srv.Serve()
+			if err = srv.Serve(); err != nil && strings.Contains(err.Error(), "use of closed network connection") {
+				err = nil
+			}
+			chErrors <- err
 		}()
 	}
 

api/server/server_linux.go

@@ -10,30 +10,8 @@ import (
 
 	"github.com/docker/docker/engine"
 	"github.com/docker/docker/pkg/systemd"
-	"net"
 )
 
-type UnixHttpServer struct {
-	srv *http.Server
-	l   net.Listener
-}
-
-func (s *UnixHttpServer) Serve() error {
-	return s.srv.Serve(s.l)
-}
-func (s *UnixHttpServer) Close() error {
-	if err := s.l.Close(); err != nil {
-		return err
-	}
-	if _, err := os.Stat(s.srv.Addr); err != nil {
-		return fmt.Errorf("Error removing unix socket %s: %s", s.srv.Addr, err.Error())
-	}
-	if err := os.Remove(s.srv.Addr); err != nil {
-		return fmt.Errorf("Error removing unix socket %s: %s", s.srv.Addr, err.Error())
-	}
-	return nil
-}
-
 // NewServer sets up the required Server and does protocol specific checking.
 func NewServer(proto, addr string, job *engine.Job) (Server, error) {
 	// Basic error and sanity checking
@@ -49,7 +27,7 @@ func NewServer(proto, addr string, job *engine.Job) (Server, error) {
 	}
 }
 
-func setupUnixHttp(addr string, job *engine.Job) (*UnixHttpServer, error) {
+func setupUnixHttp(addr string, job *engine.Job) (*HttpServer, error) {
 	r := createRouter(job.Eng, job.GetenvBool("Logging"), job.GetenvBool("EnableCors"), job.Getenv("CorsHeaders"), job.Getenv("Version"))
 
 	if err := syscall.Unlink(addr); err != nil && !os.IsNotExist(err) {
@@ -71,7 +49,7 @@ func setupUnixHttp(addr string, job *engine.Job) (*UnixHttpServer, error) {
 		return nil, err
 	}
 
-	return &UnixHttpServer{&http.Server{Addr: addr, Handler: r}, l}, nil
+	return &HttpServer{&http.Server{Addr: addr, Handler: r}, l}, nil
 }
 
 // serveFd creates an http.Server and sets it up to serve given a socket activated

docker/daemon.go

@@ -186,8 +186,9 @@ func mainDaemon() {
 	errAPI := <-serveAPIWait
 	// If we have an error here it is unique to API (as daemonErr would have
 	// exited the daemon process above)
+	eng.Shutdown()
 	if errAPI != nil {
-		log.Errorf("Shutting down due to ServeAPI error: %v", errAPI)
+		log.Fatalf("Shutting down due to ServeAPI error: %v", errAPI)
 	}
-	eng.Shutdown()
+
 }

engine/engine.go

@@ -46,18 +46,19 @@ func unregister(name string) {
 // It acts as a store for *containers*, and allows manipulation of these
 // containers by executing *jobs*.
 type Engine struct {
-	handlers   map[string]Handler
-	catchall   Handler
-	hack       Hack // data for temporary hackery (see hack.go)
-	id         string
-	Stdout     io.Writer
-	Stderr     io.Writer
-	Stdin      io.Reader
-	Logging    bool
-	tasks      sync.WaitGroup
-	l          sync.RWMutex // lock for shutdown
-	shutdown   bool
-	onShutdown []func() // shutdown handlers
+	handlers     map[string]Handler
+	catchall     Handler
+	hack         Hack // data for temporary hackery (see hack.go)
+	id           string
+	Stdout       io.Writer
+	Stderr       io.Writer
+	Stdin        io.Reader
+	Logging      bool
+	tasks        sync.WaitGroup
+	l            sync.RWMutex // lock for shutdown
+	shutdownWait sync.WaitGroup
+	shutdown     bool
+	onShutdown   []func() // shutdown handlers
 }
 
 func (eng *Engine) Register(name string, handler Handler) error {
@@ -143,6 +144,7 @@ func (eng *Engine) Job(name string, args ...string) *Job {
 func (eng *Engine) OnShutdown(h func()) {
 	eng.l.Lock()
 	eng.onShutdown = append(eng.onShutdown, h)
+	eng.shutdownWait.Add(1)
 	eng.l.Unlock()
 }
 
@@ -156,6 +158,7 @@ func (eng *Engine) Shutdown() {
 	eng.l.Lock()
 	if eng.shutdown {
 		eng.l.Unlock()
+		eng.shutdownWait.Wait()
 		return
 	}
 	eng.shutdown = true
@@ -180,17 +183,15 @@ func (eng *Engine) Shutdown() {
 
 	// Call shutdown handlers, if any.
 	// Timeout after 10 seconds.
-	var wg sync.WaitGroup
 	for _, h := range eng.onShutdown {
-		wg.Add(1)
 		go func(h func()) {
-			defer wg.Done()
 			h()
+			eng.shutdownWait.Done()
 		}(h)
 	}
 	done := make(chan struct{})
 	go func() {
-		wg.Wait()
+		eng.shutdownWait.Wait()
 		close(done)
 	}()
 	select {

integration-cli/docker_cli_daemon_test.go

@@ -800,3 +800,31 @@ func TestDaemonDots(t *testing.T) {
 
 	logDone("daemon - test dots on INFO")
 }
+
+func TestDaemonUnixSockCleanedUp(t *testing.T) {
+	d := NewDaemon(t)
+	dir, err := ioutil.TempDir("", "socket-cleanup-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	sockPath := filepath.Join(dir, "docker.sock")
+	if err := d.Start("--host", "unix://"+sockPath); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := os.Stat(sockPath); err != nil {
+		t.Fatal("socket does not exist")
+	}
+
+	if err := d.Stop(); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := os.Stat(sockPath); err == nil || !os.IsNotExist(err) {
+		t.Fatal("unix socket is not cleaned up")
+	}
+
+	logDone("daemon - unix socket is cleaned up")
+}