Ensure shutdown handler access is syncronized

There was a potential race where two handlers could be added at
the same time. Go Maps are not thread-safe, so that could do
unpleasant things. Add a mutex to keep things safe.

Also, swap the order or Register and Start for the handlers in
Libpod runtime created. As written, there was a small gap between
Start and Register where SIGTERM/SIGINT would be completely
ignored, instead of stopping Podman. Swapping the two closes this
gap.

Signed-off-by: Matthew Heon <mheon@redhat.com>

3 files changed, 21 insertions, 11 deletions

diff --git a/libpod/runtime.go b/libpod/runtime.go
index 34c737a67..0dc220b52 100644
--- a/libpod/runtime.go
+++ b/libpod/runtime.go
@@ -180,6 +180,13 @@ func newRuntimeFromConfig(ctx context.Context, conf *config.Config, options ...R
 		}
 	}
 
+	if err := shutdown.Register("libpod", func(sig os.Signal) error {
+		os.Exit(1)
+		return nil
+	}); err != nil && errors.Cause(err) != shutdown.ErrHandlerExists {
+		logrus.Errorf("Error registering shutdown handler for libpod: %v", err)
+	}
+
 	if err := shutdown.Start(); err != nil {
 		return nil, errors.Wrapf(err, "error starting shutdown signal handler")
 	}
@@ -188,13 +195,6 @@ func newRuntimeFromConfig(ctx context.Context, conf *config.Config, options ...R
 		return nil, err
 	}
 
-	if err := shutdown.Register("libpod", func(sig os.Signal) error {
-		os.Exit(1)
-		return nil
-	}); err != nil && errors.Cause(err) != shutdown.ErrHandlerExists {
-		logrus.Errorf("Error registering shutdown handler for libpod: %v", err)
-	}
-
 	return runtime, nil
 }
 
diff --git a/libpod/shutdown/handler.go b/libpod/shutdown/handler.go
index f0f228b19..ac1d33910 100644
--- a/libpod/shutdown/handler.go
+++ b/libpod/shutdown/handler.go
@@ -18,6 +18,8 @@ var (
 	stopped    bool
 	sigChan    chan os.Signal
 	cancelChan chan bool
+	// Syncronize accesses to the map
+	handlerLock sync.Mutex
 	// Definitions of all on-shutdown handlers
 	handlers map[string]func(os.Signal) error
 	// Ordering that on-shutdown handlers will be invoked.
@@ -50,6 +52,7 @@ func Start() error {
 		case sig := <-sigChan:
 			logrus.Infof("Received shutdown signal %v, terminating!", sig)
 			shutdownInhibit.Lock()
+			handlerLock.Lock()
 			for _, name := range handlerOrder {
 				handler, ok := handlers[name]
 				if !ok {
@@ -61,6 +64,7 @@ func Start() error {
 					logrus.Errorf("Error running shutdown handler %s: %v", name, err)
 				}
 			}
+			handlerLock.Unlock()
 			shutdownInhibit.Unlock()
 			return
 		}
@@ -97,6 +101,9 @@ func Uninhibit() {
 // by a signal. Handlers are invoked LIFO - the last handler registered is the
 // first run.
 func Register(name string, handler func(os.Signal) error) error {
+	handlerLock.Lock()
+	defer handlerLock.Unlock()
+
 	if handlers == nil {
 		handlers = make(map[string]func(os.Signal) error)
 	}
@@ -113,6 +120,9 @@ func Register(name string, handler func(os.Signal) error) error {
 
 // Unregister un-registers a given shutdown handler.
 func Unregister(name string) error {
+	handlerLock.Lock()
+	defer handlerLock.Unlock()
+
 	if handlers == nil {
 		return nil
 	}
diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go
index 046f6561c..d612041f6 100644
--- a/pkg/api/server/server.go
+++ b/pkg/api/server/server.go
@@ -179,15 +179,15 @@ func setupSystemd() {
 func (s *APIServer) Serve() error {
 	setupSystemd()
 
-	// Start the shutdown signal handler.
-	if err := shutdown.Start(); err != nil {
-		return err
-	}
 	if err := shutdown.Register("server", func(sig os.Signal) error {
 		return s.Shutdown()
 	}); err != nil {
 		return err
 	}
+	// Start the shutdown signal handler.
+	if err := shutdown.Start(); err != nil {
+		return err
+	}
 
 	errChan := make(chan error, 1)