From 1d9efd0e39a9663bb2fbf52b3353fe21ac3b6954 Mon Sep 17 00:00:00 2001 From: Chris Date: Thu, 11 Jan 2018 15:23:41 -0600 Subject: Remove global config watcher (#8080) * remove global config watcher * keep config watcher disabled for tests * compile fix * fix resource leak --- utils/config.go | 166 +++++++++++++++++++++++++------------------------------- 1 file changed, 75 insertions(+), 91 deletions(-) (limited to 'utils/config.go') diff --git a/utils/config.go b/utils/config.go index d1522538d..a692d82d0 100644 --- a/utils/config.go +++ b/utils/config.go @@ -18,6 +18,7 @@ import ( l4g "github.com/alecthomas/log4go" "github.com/fsnotify/fsnotify" + "github.com/pkg/errors" "github.com/spf13/viper" "net/http" @@ -35,11 +36,9 @@ const ( ) var cfgMutex = &sync.Mutex{} -var watcher *fsnotify.Watcher var Cfg *model.Config = &model.Config{} var CfgHash = "" var CfgFileName string = "" -var CfgDisableConfigWatch = false var originalDisableDebugLvl l4g.Level = l4g.DEBUG var siteURL = "" @@ -201,78 +200,61 @@ func SaveConfig(fileName string, config *model.Config) *model.AppError { return nil } -func InitializeConfigWatch() { - cfgMutex.Lock() - defer cfgMutex.Unlock() +type ConfigWatcher struct { + watcher *fsnotify.Watcher + close chan struct{} + closed chan struct{} +} - if CfgDisableConfigWatch { - return +func NewConfigWatcher(cfgFileName string) (*ConfigWatcher, error) { + watcher, err := fsnotify.NewWatcher() + if err != nil { + return nil, errors.Wrapf(err, "failed to create config watcher for file: "+cfgFileName) } - if watcher == nil { - var err error - watcher, err = fsnotify.NewWatcher() - if err != nil { - l4g.Error(fmt.Sprintf("Failed to watch config file at %v with err=%v", CfgFileName, err.Error())) - } + configFile := filepath.Clean(cfgFileName) + configDir, _ := filepath.Split(configFile) + watcher.Add(configDir) - go func() { - configFile := filepath.Clean(CfgFileName) - - for { - select { - case event := <-watcher.Events: - // we only care about the config file - if filepath.Clean(event.Name) == configFile { - if event.Op&fsnotify.Write == fsnotify.Write || event.Op&fsnotify.Create == fsnotify.Create { - l4g.Info(fmt.Sprintf("Config file watcher detected a change reloading %v", CfgFileName)) - - if _, configReadErr := ReadConfigFile(CfgFileName, true); configReadErr == nil { - LoadGlobalConfig(CfgFileName) - } else { - l4g.Error(fmt.Sprintf("Failed to read while watching config file at %v with err=%v", CfgFileName, configReadErr.Error())) - } - } - } - case err := <-watcher.Errors: - l4g.Error(fmt.Sprintf("Failed while watching config file at %v with err=%v", CfgFileName, err.Error())) - } - } - }() + ret := &ConfigWatcher{ + watcher: watcher, + close: make(chan struct{}), + closed: make(chan struct{}), } -} -func EnableConfigWatch() { - cfgMutex.Lock() - defer cfgMutex.Unlock() + go func() { + defer close(ret.closed) + defer watcher.Close() - if watcher != nil { - configFile := filepath.Clean(CfgFileName) - configDir, _ := filepath.Split(configFile) + for { + select { + case event := <-watcher.Events: + // we only care about the config file + if filepath.Clean(event.Name) == configFile { + if event.Op&fsnotify.Write == fsnotify.Write || event.Op&fsnotify.Create == fsnotify.Create { + l4g.Info(fmt.Sprintf("Config file watcher detected a change reloading %v", cfgFileName)) - if watcher != nil { - watcher.Add(configDir) + if _, configReadErr := ReadConfigFile(cfgFileName, true); configReadErr == nil { + LoadGlobalConfig(cfgFileName) + } else { + l4g.Error(fmt.Sprintf("Failed to read while watching config file at %v with err=%v", cfgFileName, configReadErr.Error())) + } + } + } + case err := <-watcher.Errors: + l4g.Error(fmt.Sprintf("Failed while watching config file at %v with err=%v", cfgFileName, err.Error())) + case <-ret.close: + return + } } - } -} - -func DisableConfigWatch() { - cfgMutex.Lock() - defer cfgMutex.Unlock() + }() - if watcher != nil { - configFile := filepath.Clean(CfgFileName) - configDir, _ := filepath.Split(configFile) - watcher.Remove(configDir) - } + return ret, nil } -func InitAndLoadConfig(filename string) error { - LoadGlobalConfig(filename) - InitializeConfigWatch() - EnableConfigWatch() - - return nil +func (w *ConfigWatcher) Close() { + close(w.close) + <-w.closed } // ReadConfig reads and parses the given configuration. @@ -337,26 +319,16 @@ func EnsureConfigFile(fileName string) (string, error) { return "", fmt.Errorf("no config file found") } -// LoadGlobalConfig will try to search around for the corresponding config file. It will search +// LoadConfig will try to search around for the corresponding config file. It will search // /tmp/fileName then attempt ./config/fileName, then ../config/fileName and last it will look at -// fileName -// -// XXX: This is deprecated. -func LoadGlobalConfig(fileName string) *model.Config { - cfgMutex.Lock() - defer cfgMutex.Unlock() - - // Cfg should never be null - oldConfig := *Cfg - - var configPath string +// fileName. +func LoadConfig(fileName string) (configPath string, config *model.Config, appErr *model.AppError) { if fileName != filepath.Base(fileName) { configPath = fileName } else { if path, err := EnsureConfigFile(fileName); err != nil { - errMsg := T("utils.config.load_config.opening.panic", map[string]interface{}{"Filename": fileName, "Error": err.Error()}) - fmt.Fprintln(os.Stderr, errMsg) - os.Exit(1) + appErr = model.NewAppError("LoadConfig", "utils.config.load_config.opening.panic", map[string]interface{}{"Filename": fileName, "Error": err.Error()}, "", 0) + return } else { configPath = path } @@ -364,40 +336,31 @@ func LoadGlobalConfig(fileName string) *model.Config { config, err := ReadConfigFile(configPath, true) if err != nil { - errMsg := T("utils.config.load_config.decoding.panic", map[string]interface{}{"Filename": fileName, "Error": err.Error()}) - fmt.Fprintln(os.Stderr, errMsg) - os.Exit(1) + appErr = model.NewAppError("LoadConfig", "utils.config.load_config.decoding.panic", map[string]interface{}{"Filename": fileName, "Error": err.Error()}, "", 0) + return } - CfgFileName = configPath - needSave := len(config.SqlSettings.AtRestEncryptKey) == 0 || len(*config.FileSettings.PublicLinkSalt) == 0 || len(config.EmailSettings.InviteSalt) == 0 config.SetDefaults() if err := config.IsValid(); err != nil { - panic(err.Message) + return "", nil, err } if needSave { - cfgMutex.Unlock() - if err := SaveConfig(CfgFileName, config); err != nil { + if err := SaveConfig(configPath, config); err != nil { l4g.Warn(err.Error()) } - cfgMutex.Lock() } if err := ValidateLocales(config); err != nil { - cfgMutex.Unlock() - if err := SaveConfig(CfgFileName, config); err != nil { + if err := SaveConfig(configPath, config); err != nil { l4g.Warn(err.Error()) } - cfgMutex.Lock() } - configureLog(&config.LogSettings) - if *config.FileSettings.DriverName == model.IMAGE_DRIVER_LOCAL { dir := config.FileSettings.Directory if len(dir) > 0 && dir[len(dir)-1:] != "/" { @@ -405,6 +368,27 @@ func LoadGlobalConfig(fileName string) *model.Config { } } + return configPath, config, nil +} + +// XXX: This is deprecated. Use LoadConfig instead if possible. +func LoadGlobalConfig(fileName string) *model.Config { + configPath, config, err := LoadConfig(fileName) + if err != nil { + fmt.Fprintln(os.Stderr, err.SystemMessage(T)) + os.Exit(1) + } + + cfgMutex.Lock() + defer cfgMutex.Unlock() + + CfgFileName = configPath + + configureLog(&config.LogSettings) + + // Cfg should never be null + oldConfig := *Cfg + Cfg = config CfgHash = fmt.Sprintf("%x", md5.Sum([]byte(Cfg.ToJson()))) -- cgit v1.2.3-1-g7c22