config: move locking to fix fatal error: concurrent map read and map write
Before this change we assumed that github.com/Unknwon/goconfig was threadsafe as documented. However it turns out it is not threadsafe and looking at the code it appears that making it threadsafe might be quite hard. So this change increases the lock coverage in configfile to cover the goconfig uses also. Fixes #6378
This commit is contained in:
parent
d08ed7d1e9
commit
3cb7734eac
1 changed files with 36 additions and 13 deletions
|
@ -24,16 +24,15 @@ func Install() {
|
||||||
// Storage implements config.Storage for saving and loading config
|
// Storage implements config.Storage for saving and loading config
|
||||||
// data in a simple INI based file.
|
// data in a simple INI based file.
|
||||||
type Storage struct {
|
type Storage struct {
|
||||||
gc *goconfig.ConfigFile // config file loaded - thread safe
|
|
||||||
mu sync.Mutex // to protect the following variables
|
mu sync.Mutex // to protect the following variables
|
||||||
|
gc *goconfig.ConfigFile // config file loaded - not thread safe
|
||||||
fi os.FileInfo // stat of the file when last loaded
|
fi os.FileInfo // stat of the file when last loaded
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check to see if we need to reload the config
|
// Check to see if we need to reload the config
|
||||||
func (s *Storage) check() {
|
//
|
||||||
s.mu.Lock()
|
// mu must be held when calling this
|
||||||
defer s.mu.Unlock()
|
func (s *Storage) _check() {
|
||||||
|
|
||||||
if configPath := config.GetConfigPath(); configPath != "" {
|
if configPath := config.GetConfigPath(); configPath != "" {
|
||||||
// Check to see if config file has changed since it was last loaded
|
// Check to see if config file has changed since it was last loaded
|
||||||
fi, err := os.Stat(configPath)
|
fi, err := os.Stat(configPath)
|
||||||
|
@ -174,7 +173,10 @@ func (s *Storage) Save() error {
|
||||||
|
|
||||||
// Serialize the config into a string
|
// Serialize the config into a string
|
||||||
func (s *Storage) Serialize() (string, error) {
|
func (s *Storage) Serialize() (string, error) {
|
||||||
s.check()
|
s.mu.Lock()
|
||||||
|
defer s.mu.Unlock()
|
||||||
|
|
||||||
|
s._check()
|
||||||
var buf bytes.Buffer
|
var buf bytes.Buffer
|
||||||
if err := goconfig.SaveConfigData(s.gc, &buf); err != nil {
|
if err := goconfig.SaveConfigData(s.gc, &buf); err != nil {
|
||||||
return "", fmt.Errorf("failed to save config file: %w", err)
|
return "", fmt.Errorf("failed to save config file: %w", err)
|
||||||
|
@ -185,7 +187,10 @@ func (s *Storage) Serialize() (string, error) {
|
||||||
|
|
||||||
// HasSection returns true if section exists in the config file
|
// HasSection returns true if section exists in the config file
|
||||||
func (s *Storage) HasSection(section string) bool {
|
func (s *Storage) HasSection(section string) bool {
|
||||||
s.check()
|
s.mu.Lock()
|
||||||
|
defer s.mu.Unlock()
|
||||||
|
|
||||||
|
s._check()
|
||||||
_, err := s.gc.GetSection(section)
|
_, err := s.gc.GetSection(section)
|
||||||
return err == nil
|
return err == nil
|
||||||
}
|
}
|
||||||
|
@ -193,26 +198,38 @@ func (s *Storage) HasSection(section string) bool {
|
||||||
// DeleteSection removes the named section and all config from the
|
// DeleteSection removes the named section and all config from the
|
||||||
// config file
|
// config file
|
||||||
func (s *Storage) DeleteSection(section string) {
|
func (s *Storage) DeleteSection(section string) {
|
||||||
s.check()
|
s.mu.Lock()
|
||||||
|
defer s.mu.Unlock()
|
||||||
|
|
||||||
|
s._check()
|
||||||
s.gc.DeleteSection(section)
|
s.gc.DeleteSection(section)
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetSectionList returns a slice of strings with names for all the
|
// GetSectionList returns a slice of strings with names for all the
|
||||||
// sections
|
// sections
|
||||||
func (s *Storage) GetSectionList() []string {
|
func (s *Storage) GetSectionList() []string {
|
||||||
s.check()
|
s.mu.Lock()
|
||||||
|
defer s.mu.Unlock()
|
||||||
|
|
||||||
|
s._check()
|
||||||
return s.gc.GetSectionList()
|
return s.gc.GetSectionList()
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetKeyList returns the keys in this section
|
// GetKeyList returns the keys in this section
|
||||||
func (s *Storage) GetKeyList(section string) []string {
|
func (s *Storage) GetKeyList(section string) []string {
|
||||||
s.check()
|
s.mu.Lock()
|
||||||
|
defer s.mu.Unlock()
|
||||||
|
|
||||||
|
s._check()
|
||||||
return s.gc.GetKeyList(section)
|
return s.gc.GetKeyList(section)
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetValue returns the key in section with a found flag
|
// GetValue returns the key in section with a found flag
|
||||||
func (s *Storage) GetValue(section string, key string) (value string, found bool) {
|
func (s *Storage) GetValue(section string, key string) (value string, found bool) {
|
||||||
s.check()
|
s.mu.Lock()
|
||||||
|
defer s.mu.Unlock()
|
||||||
|
|
||||||
|
s._check()
|
||||||
value, err := s.gc.GetValue(section, key)
|
value, err := s.gc.GetValue(section, key)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", false
|
return "", false
|
||||||
|
@ -222,7 +239,10 @@ func (s *Storage) GetValue(section string, key string) (value string, found bool
|
||||||
|
|
||||||
// SetValue sets the value under key in section
|
// SetValue sets the value under key in section
|
||||||
func (s *Storage) SetValue(section string, key string, value string) {
|
func (s *Storage) SetValue(section string, key string, value string) {
|
||||||
s.check()
|
s.mu.Lock()
|
||||||
|
defer s.mu.Unlock()
|
||||||
|
|
||||||
|
s._check()
|
||||||
if strings.HasPrefix(section, ":") {
|
if strings.HasPrefix(section, ":") {
|
||||||
fs.Logf(nil, "Can't save config %q for on the fly backend %q", key, section)
|
fs.Logf(nil, "Can't save config %q for on the fly backend %q", key, section)
|
||||||
return
|
return
|
||||||
|
@ -232,7 +252,10 @@ func (s *Storage) SetValue(section string, key string, value string) {
|
||||||
|
|
||||||
// DeleteKey removes the key under section
|
// DeleteKey removes the key under section
|
||||||
func (s *Storage) DeleteKey(section string, key string) bool {
|
func (s *Storage) DeleteKey(section string, key string) bool {
|
||||||
s.check()
|
s.mu.Lock()
|
||||||
|
defer s.mu.Unlock()
|
||||||
|
|
||||||
|
s._check()
|
||||||
return s.gc.DeleteKey(section, key)
|
return s.gc.DeleteKey(section, key)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue