Skip to content

Commit

Permalink
fix: config deadlock by trying to acquire a read lock twice
Browse files Browse the repository at this point in the history
  • Loading branch information
@atzoum
atzoum committed Sep 15, 2023
1 parent fda5211 commit 677a087
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 26 deletions.
25 changes: 15 additions & 10 deletions config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func GetBool(key string, defaultValue bool) (value bool) {
func (c *Config) GetBool(key string, defaultValue bool) (value bool) {
c.vLock.RLock()
defer c.vLock.RUnlock()
if !c.IsSet(key) {
if !c.isSetInternal(key) {
return defaultValue
}
return c.v.GetBool(key)
Expand All @@ -126,7 +126,7 @@ func GetInt(key string, defaultValue int) (value int) {
func (c *Config) GetInt(key string, defaultValue int) (value int) {
c.vLock.RLock()
defer c.vLock.RUnlock()
if !c.IsSet(key) {
if !c.isSetInternal(key) {
return defaultValue
}
return c.v.GetInt(key)
Expand All @@ -141,7 +141,7 @@ func GetStringMap(key string, defaultValue map[string]interface{}) (value map[st
func (c *Config) GetStringMap(key string, defaultValue map[string]interface{}) (value map[string]interface{}) {
c.vLock.RLock()
defer c.vLock.RUnlock()
if !c.IsSet(key) {
if !c.isSetInternal(key) {
return defaultValue
}
return c.v.GetStringMap(key)
Expand All @@ -156,7 +156,7 @@ func MustGetInt(key string) (value int) {
func (c *Config) MustGetInt(key string) (value int) {
c.vLock.RLock()
defer c.vLock.RUnlock()
if !c.IsSet(key) {
if !c.isSetInternal(key) {
panic(fmt.Errorf("config key %s not found", key))
}
return c.v.GetInt(key)
Expand All @@ -171,7 +171,7 @@ func GetInt64(key string, defaultValue int64) (value int64) {
func (c *Config) GetInt64(key string, defaultValue int64) (value int64) {
c.vLock.RLock()
defer c.vLock.RUnlock()
if !c.IsSet(key) {
if !c.isSetInternal(key) {
return defaultValue
}
return c.v.GetInt64(key)
Expand All @@ -186,7 +186,7 @@ func GetFloat64(key string, defaultValue float64) (value float64) {
func (c *Config) GetFloat64(key string, defaultValue float64) (value float64) {
c.vLock.RLock()
defer c.vLock.RUnlock()
if !c.IsSet(key) {
if !c.isSetInternal(key) {
return defaultValue
}
return c.v.GetFloat64(key)
Expand All @@ -201,7 +201,7 @@ func GetString(key, defaultValue string) (value string) {
func (c *Config) GetString(key, defaultValue string) (value string) {
c.vLock.RLock()
defer c.vLock.RUnlock()
if !c.IsSet(key) {
if !c.isSetInternal(key) {
return defaultValue
}
return c.v.GetString(key)
Expand All @@ -216,7 +216,7 @@ func MustGetString(key string) (value string) {
func (c *Config) MustGetString(key string) (value string) {
c.vLock.RLock()
defer c.vLock.RUnlock()
if !c.IsSet(key) {
if !c.isSetInternal(key) {
panic(fmt.Errorf("config key %s not found", key))
}
return c.v.GetString(key)
Expand All @@ -231,7 +231,7 @@ func GetStringSlice(key string, defaultValue []string) (value []string) {
func (c *Config) GetStringSlice(key string, defaultValue []string) (value []string) {
c.vLock.RLock()
defer c.vLock.RUnlock()
if !c.IsSet(key) {
if !c.isSetInternal(key) {
return defaultValue
}
return c.v.GetStringSlice(key)
Expand All @@ -246,7 +246,7 @@ func GetDuration(key string, defaultValueInTimescaleUnits int64, timeScale time.
func (c *Config) GetDuration(key string, defaultValueInTimescaleUnits int64, timeScale time.Duration) (value time.Duration) {
c.vLock.RLock()
defer c.vLock.RUnlock()
if !c.IsSet(key) {
if !c.isSetInternal(key) {
return time.Duration(defaultValueInTimescaleUnits) * timeScale
} else {
v := c.v.GetString(key)
Expand All @@ -273,6 +273,11 @@ func IsSet(key string) bool {
func (c *Config) IsSet(key string) bool {
c.vLock.RLock()
defer c.vLock.RUnlock()
return c.isSetInternal(key)
}

// isSetInternal checks if config is set for a key. Caller needs to hold a read lock on vLock.
func (c *Config) isSetInternal(key string) bool {
c.bindEnv(key)
return c.v.IsSet(key)
}
Expand Down
16 changes: 8 additions & 8 deletions config/hotreloadable.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (c *Config) registerIntVar(
}

for _, key := range orderedKeys {
if c.IsSet(key) {
if c.isSetInternal(key) {
store(c.GetInt(key, defaultValue) * valueScale)
return
}
Expand Down Expand Up @@ -180,7 +180,7 @@ func (c *Config) registerBoolVar(defaultValue bool, ptr any, isHotReloadable boo

for _, key := range orderedKeys {
c.bindEnv(key)
if c.IsSet(key) {
if c.isSetInternal(key) {
store(c.GetBool(key, defaultValue))
return
}
Expand Down Expand Up @@ -274,7 +274,7 @@ func (c *Config) registerFloat64Var(

for _, key := range orderedKeys {
c.bindEnv(key)
if c.IsSet(key) {
if c.isSetInternal(key) {
store(c.GetFloat64(key, defaultValue))
return
}
Expand Down Expand Up @@ -368,7 +368,7 @@ func (c *Config) registerInt64Var(

for _, key := range orderedKeys {
c.bindEnv(key)
if c.IsSet(key) {
if c.isSetInternal(key) {
store(c.GetInt64(key, defaultValue) * valueScale)
return
}
Expand Down Expand Up @@ -471,7 +471,7 @@ func (c *Config) registerDurationVar(
}

for _, key := range orderedKeys {
if c.IsSet(key) {
if c.isSetInternal(key) {
store(c.GetDuration(key, defaultValueInTimescaleUnits, timeScale))
return
}
Expand Down Expand Up @@ -563,7 +563,7 @@ func (c *Config) registerStringVar(
}

for _, key := range orderedKeys {
if c.IsSet(key) {
if c.isSetInternal(key) {
store(c.GetString(key, defaultValue))
return
}
Expand Down Expand Up @@ -655,7 +655,7 @@ func (c *Config) registerStringSliceVar(
}

for _, key := range orderedKeys {
if c.IsSet(key) {
if c.isSetInternal(key) {
store(c.GetStringSlice(key, defaultValue))
return
}
Expand Down Expand Up @@ -755,7 +755,7 @@ func (c *Config) registerStringMapVar(
}

for _, key := range orderedKeys {
if c.IsSet(key) {
if c.isSetInternal(key) {
store(c.GetStringMap(key, defaultValue))
return
}
Expand Down
16 changes: 8 additions & 8 deletions config/load.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value int
var isSet bool
for _, key := range configVal.keys {
if c.IsSet(key) {
if c.isSetInternal(key) {
isSet = true
_value = c.GetInt(key, configVal.defaultValue.(int))
break
Expand All @@ -79,7 +79,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value int64
var isSet bool
for _, key := range configVal.keys {
if c.IsSet(key) {
if c.isSetInternal(key) {
isSet = true
_value = c.GetInt64(key, configVal.defaultValue.(int64))
break
Expand All @@ -94,7 +94,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value string
var isSet bool
for _, key := range configVal.keys {
if c.IsSet(key) {
if c.isSetInternal(key) {
isSet = true
_value = c.GetString(key, configVal.defaultValue.(string))
break
Expand All @@ -108,7 +108,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value time.Duration
var isSet bool
for _, key := range configVal.keys {
if c.IsSet(key) {
if c.isSetInternal(key) {
isSet = true
_value = c.GetDuration(key, configVal.defaultValue.(int64), configVal.multiplier.(time.Duration))
break
Expand All @@ -122,7 +122,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value bool
var isSet bool
for _, key := range configVal.keys {
if c.IsSet(key) {
if c.isSetInternal(key) {
isSet = true
_value = c.GetBool(key, configVal.defaultValue.(bool))
break
Expand All @@ -136,7 +136,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value float64
var isSet bool
for _, key := range configVal.keys {
if c.IsSet(key) {
if c.isSetInternal(key) {
isSet = true
_value = c.GetFloat64(key, configVal.defaultValue.(float64))
break
Expand All @@ -151,7 +151,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value []string
var isSet bool
for _, key := range configVal.keys {
if c.IsSet(key) {
if c.isSetInternal(key) {
isSet = true
_value = c.GetStringSlice(key, configVal.defaultValue.([]string))
break
Expand All @@ -167,7 +167,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value map[string]interface{}
var isSet bool
for _, key := range configVal.keys {
if c.IsSet(key) {
if c.isSetInternal(key) {
isSet = true
_value = c.GetStringMap(key, configVal.defaultValue.(map[string]interface{}))
break
Expand Down

0 comments on commit 677a087

Please sign in to comment.