From 5526ac13d2908aba8a7000570dbf9ec7cffea951 Mon Sep 17 00:00:00 2001 From: Anis Elleuch Date: Mon, 15 Aug 2016 07:55:48 +0100 Subject: [PATCH] Protect shutdown callbacks lists with a mutex (#2432) --- .travis.yml | 9 +--- fs-v1.go | 65 +++++++++++++-------------- main.go | 7 --- object-interface.go | 1 + routers.go | 15 +++++++ server-main.go | 3 +- utils.go | 107 ++++++++++++++++++++++++++++++++------------ utils_test.go | 22 +++++---- xl-v1.go | 6 +++ 9 files changed, 144 insertions(+), 91 deletions(-) diff --git a/.travis.yml b/.travis.yml index 98e78227c..d046c1b7a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,12 +19,5 @@ script: after_success: - bash <(curl -s https://codecov.io/bash) -after_success: -- bash <(curl -s https://codecov.io/bash) - go: -- 1.6 - -notifications: - slack: - secure: K9tsn5MvrCAxuEZTxn+m3Kq1K2NG2xMEJFSv/sTp+RQBW7TslPHzv859GsIvrm8mU1y1btOU9RlOzqrRUczI5cJpE8IL1oljPZbXrIXgetE0kbsw0Wpy99g27UQ2VGp933WDu8tfj7zU4cZv+BI0RltNLwqYO6GWXmcWP0IueCU= +- 1.6.2 diff --git a/fs-v1.go b/fs-v1.go index ca299f50b..8a4261e6e 100644 --- a/fs-v1.go +++ b/fs-v1.go @@ -62,36 +62,6 @@ func loadFormatFS(storageDisk StorageAPI) (format formatConfigV1, err error) { return format, nil } -// Should be called when process shuts down. -func shutdownFS(storage StorageAPI) errCode { - // List if there are any multipart entries. - _, err := storage.ListDir(minioMetaBucket, mpartMetaPrefix) - if err != errFileNotFound { - // Multipart directory is not empty hence do not remove '.minio.sys' volume. - return exitSuccess - } - // List if there are any bucket configuration entries. - _, err = storage.ListDir(minioMetaBucket, bucketConfigPrefix) - if err != errFileNotFound { - // Bucket config directory is not empty hence do not remove '.minio.sys' volume. - return exitSuccess - } - // Cleanup everything else. - prefix := "" - if err = cleanupDir(storage, minioMetaBucket, prefix); err != nil { - errorIf(err, "Unable to cleanup minio meta bucket") - return exitFailure - } - if err = storage.DeleteVol(minioMetaBucket); err != nil { - if err != errVolumeNotEmpty { - errorIf(err, "Unable to delete minio meta bucket %s", minioMetaBucket) - return exitFailure - } - } - // Successful exit. - return exitSuccess -} - // newFSObjects - initialize new fs object layer. func newFSObjects(disk string) (ObjectLayer, error) { storage, err := newStorageAPI(disk) @@ -132,11 +102,6 @@ func newFSObjects(disk string) (ObjectLayer, error) { return nil, errFSDiskFormat } - // Register the callback that should be called when the process shuts down. - registerObjectStorageShutdown(func() errCode { - return shutdownFS(storage) - }) - // Initialize fs objects. fs := fsObjects{ storage: storage, @@ -148,6 +113,36 @@ func newFSObjects(disk string) (ObjectLayer, error) { return fs, nil } +// Should be called when process shuts down. +func (fs fsObjects) Shutdown() error { + // List if there are any multipart entries. + _, err := fs.storage.ListDir(minioMetaBucket, mpartMetaPrefix) + if err != errFileNotFound { + // Multipart directory is not empty hence do not remove '.minio.sys' volume. + return nil + } + // List if there are any bucket configuration entries. + _, err = fs.storage.ListDir(minioMetaBucket, bucketConfigPrefix) + if err != errFileNotFound { + // Bucket config directory is not empty hence do not remove '.minio.sys' volume. + return nil + } + // Cleanup everything else. + prefix := "" + if err = cleanupDir(fs.storage, minioMetaBucket, prefix); err != nil { + errorIf(err, "Unable to cleanup minio meta bucket") + return err + } + if err = fs.storage.DeleteVol(minioMetaBucket); err != nil { + if err != errVolumeNotEmpty { + errorIf(err, "Unable to delete minio meta bucket %s", minioMetaBucket) + return err + } + } + // Successful. + return nil +} + // StorageInfo - returns underlying storage statistics. func (fs fsObjects) StorageInfo() StorageInfo { info, err := disk.GetInfo(fs.physicalDisk) diff --git a/main.go b/main.go index 83e6df11d..0e6de9306 100644 --- a/main.go +++ b/main.go @@ -158,9 +158,6 @@ func main() { // Enable all loggers by now. enableLoggers() - // Initialize name space lock. - initNSLock() - // Set global quiet flag. globalQuiet = c.Bool("quiet") || c.GlobalBool("quiet") @@ -189,10 +186,6 @@ func main() { defer profile.Start(profile.BlockProfile, profile.ProfilePath(profileDir)).Stop() } - // Initialize and monitor shutdown signal - shutdownSignal = make(chan bool, 1) - monitorShutdownSignal(os.Exit) - // Run the app - exit on error. app.RunAndExitOnError() } diff --git a/object-interface.go b/object-interface.go index 43e4b6bd5..839fab282 100644 --- a/object-interface.go +++ b/object-interface.go @@ -21,6 +21,7 @@ import "io" // ObjectLayer implements primitives for object API layer. type ObjectLayer interface { // Storage operations. + Shutdown() error StorageInfo() StorageInfo // Bucket operations. diff --git a/routers.go b/routers.go index c7b4069db..b13f6342d 100644 --- a/routers.go +++ b/routers.go @@ -42,6 +42,9 @@ func newObjectLayer(disks, ignoredDisks []string) (ObjectLayer, error) { // configureServer handler returns final handler for the http server. func configureServerHandler(srvCmdConfig serverCmdConfig) http.Handler { + // Initialize name space lock. + initNSLock() + objAPI, err := newObjectLayer(srvCmdConfig.disks, srvCmdConfig.ignoredDisks) fatalIf(err, "Unable to intialize object layer.") @@ -66,6 +69,18 @@ func configureServerHandler(srvCmdConfig serverCmdConfig) http.Handler { ObjectAPI: objAPI, } + // Initialize and monitor shutdown signals. + err = initGracefulShutdown(os.Exit) + fatalIf(err, "Unable to initialize graceful shutdown operation") + + // Register the callback that should be called when the process shuts down. + globalShutdownCBs.AddObjectLayerCB(func() errCode { + if sErr := objAPI.Shutdown(); sErr != nil { + return exitFailure + } + return exitSuccess + }) + // Initialize a new event notifier. err = initEventNotifier(objAPI) fatalIf(err, "Unable to initialize event notification queue") diff --git a/server-main.go b/server-main.go index 76d9f7596..a4e19c353 100644 --- a/server-main.go +++ b/server-main.go @@ -259,7 +259,8 @@ func serverMain(c *cli.Context) { // Prints the formatted startup message. printStartupMessage(endPoints) - registerShutdown(func() errCode { + // Register generic callbacks. + globalShutdownCBs.AddGenericCB(func() errCode { // apiServer.Stop() return exitSuccess }) diff --git a/utils.go b/utils.go index 5e8aacf0d..44314b25c 100644 --- a/utils.go +++ b/utils.go @@ -22,6 +22,7 @@ import ( "io" "os" "strings" + "sync" "syscall" ) @@ -76,62 +77,112 @@ func contains(stringList []string, element string) bool { return false } -// shutdownSignal - is the channel that receives any boolean when -// we want broadcast the start of shutdown -var shutdownSignal chan bool +// Represents a type of an exit func which will be invoked upon shutdown signal. +type onExitFunc func(code int) -// shutdownCallbacks - is the list of function callbacks executed one by one -// when a shutdown starts. A callback returns 0 for success and 1 for failure. -// Failure is considered an emergency error that needs an immediate exit -var shutdownCallbacks []func() errCode +// Represents a type for all the the callback functions invoked upon shutdown signal. +type cleanupOnExitFunc func() errCode + +// Represents a collection of various callbacks executed upon exit signals. +type shutdownCallbacks struct { + // Protect callbacks list from a concurrent access + *sync.RWMutex + // genericCallbacks - is the list of function callbacks executed one by one + // when a shutdown starts. A callback returns 0 for success and 1 for failure. + // Failure is considered an emergency error that needs an immediate exit + genericCallbacks []cleanupOnExitFunc + // objectLayerCallbacks - contains the list of function callbacks that + // need to be invoked when a shutdown starts. These callbacks will be called before + // the general callback shutdowns + objectLayerCallbacks []cleanupOnExitFunc +} -// shutdownObjectStorageCallbacks - contains the list of function callbacks that -// need to be invoked when a shutdown starts. These callbacks will be called before -// the general callback shutdowns -var shutdownObjectStorageCallbacks []func() errCode +// globalShutdownCBs stores regular and object storages callbacks +var globalShutdownCBs *shutdownCallbacks -// Register callback functions that need to be called when process terminates. -func registerShutdown(callback func() errCode) { - shutdownCallbacks = append(shutdownCallbacks, callback) +func (s shutdownCallbacks) GetObjectLayerCBs() []cleanupOnExitFunc { + s.RLock() + defer s.RUnlock() + return s.objectLayerCallbacks } -// Register object storagecallback functions that need to be called when process terminates. -func registerObjectStorageShutdown(callback func() errCode) { - shutdownObjectStorageCallbacks = append(shutdownObjectStorageCallbacks, callback) +func (s shutdownCallbacks) GetGenericCBs() []cleanupOnExitFunc { + s.RLock() + defer s.RUnlock() + return s.genericCallbacks } -// Represents a type of an exit func which will be invoked during shutdown signal. -type onExitFunc func(code int) +func (s *shutdownCallbacks) AddObjectLayerCB(callback cleanupOnExitFunc) error { + s.Lock() + defer s.Unlock() + if callback == nil { + return errInvalidArgument + } + s.objectLayerCallbacks = append(s.objectLayerCallbacks, callback) + return nil +} + +func (s *shutdownCallbacks) AddGenericCB(callback cleanupOnExitFunc) error { + s.Lock() + defer s.Unlock() + if callback == nil { + return errInvalidArgument + } + s.genericCallbacks = append(s.genericCallbacks, callback) + return nil +} + +// Initialize graceful shutdown mechanism. +func initGracefulShutdown(onExitFn onExitFunc) error { + // Validate exit func. + if onExitFn == nil { + return errInvalidArgument + } + globalShutdownCBs = &shutdownCallbacks{ + RWMutex: &sync.RWMutex{}, + } + // Return start monitor shutdown signal. + return startMonitorShutdownSignal(onExitFn) +} + +// Global shutdown signal channel. +var globalShutdownSignalCh = make(chan struct{}) // Start to monitor shutdownSignal to execute shutdown callbacks -func monitorShutdownSignal(onExitFn onExitFunc) { +func startMonitorShutdownSignal(onExitFn onExitFunc) error { + // Validate exit func. + if onExitFn == nil { + return errInvalidArgument + } go func() { + defer close(globalShutdownSignalCh) // Monitor signals. trapCh := signalTrap(os.Interrupt, syscall.SIGTERM) for { select { case <-trapCh: - // Start a graceful shutdown call - shutdownSignal <- true - case <-shutdownSignal: - // Call all callbacks and exit for emergency - for _, callback := range shutdownCallbacks { + // Initiate graceful shutdown. + globalShutdownSignalCh <- struct{}{} + case <-globalShutdownSignalCh: + // Call all object storage shutdown callbacks and exit for emergency + for _, callback := range globalShutdownCBs.GetObjectLayerCBs() { exitCode := callback() if exitCode != exitSuccess { onExitFn(int(exitCode)) } } - // Call all object storage shutdown callbacks and exit for emergency - for _, callback := range shutdownObjectStorageCallbacks { + // Call all callbacks and exit for emergency + for _, callback := range globalShutdownCBs.GetGenericCBs() { exitCode := callback() if exitCode != exitSuccess { onExitFn(int(exitCode)) } - } onExitFn(int(exitSuccess)) } } }() + // Successfully started routine. + return nil } diff --git a/utils_test.go b/utils_test.go index 76a5ed318..c45aa3bfa 100644 --- a/utils_test.go +++ b/utils_test.go @@ -20,21 +20,19 @@ import "testing" // ShutdownCallback simulates a successful and failure exit here. func TestShutdownCallbackSuccess(t *testing.T) { - // Register two callbacks that return success - registerObjectStorageShutdown(func() errCode { - return exitSuccess - }) - registerShutdown(func() errCode { - return exitSuccess - }) - - shutdownSignal = make(chan bool, 1) - shutdownSignal <- true - // Start executing callbacks and exitFunc receives a success. + // initialize graceful shutdown dummySuccess := func(code int) { if code != int(exitSuccess) { t.Fatalf("Expected %d, got %d instead.", code, exitSuccess) } } - monitorShutdownSignal(dummySuccess) + initGracefulShutdown(dummySuccess) + // Register two callbacks that return success + globalShutdownCBs.AddObjectLayerCB(func() errCode { + return exitSuccess + }) + globalShutdownCBs.AddGenericCB(func() errCode { + return exitSuccess + }) + globalShutdownSignalCh <- struct{}{} } diff --git a/xl-v1.go b/xl-v1.go index 8967e7302..a15027bf2 100644 --- a/xl-v1.go +++ b/xl-v1.go @@ -206,6 +206,12 @@ func newXLObjects(disks, ignoredDisks []string) (ObjectLayer, error) { return xl, nil } +// Shutdown function for object storage interface. +func (xl xlObjects) Shutdown() error { + // Add any object layer shutdown activities here. + return nil +} + // byDiskTotal is a collection satisfying sort.Interface. type byDiskTotal []disk.Info