Skip to content

Commit

Permalink
fix: another 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 ac71452 commit 75af0a6
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 50 deletions.
64 changes: 24 additions & 40 deletions config/hotreloadable.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,6 @@ func (c *Config) GetReloadableIntVar(defaultValue, valueScale int, orderedKeys .
func (c *Config) registerIntVar(
defaultValue int, ptr any, isHotReloadable bool, valueScale int, store func(int), orderedKeys ...string,
) {
c.vLock.RLock()
defer c.vLock.RUnlock()
c.hotReloadableConfigLock.Lock()
defer c.hotReloadableConfigLock.Unlock()
configVar := configValue{
value: ptr,
multiplier: valueScale,
Expand All @@ -87,11 +83,13 @@ func (c *Config) registerIntVar(
}

if isHotReloadable {
c.hotReloadableConfigLock.Lock()
c.appendVarToConfigMaps(orderedKeys, &configVar)
c.hotReloadableConfigLock.Unlock()
}

for _, key := range orderedKeys {
if c.isSetInternal(key) {
if c.IsSet(key) {
store(c.GetInt(key, defaultValue) * valueScale)
return
}
Expand Down Expand Up @@ -164,23 +162,21 @@ func (c *Config) GetReloadableBoolVar(defaultValue bool, orderedKeys ...string)
}

func (c *Config) registerBoolVar(defaultValue bool, ptr any, isHotReloadable bool, store func(bool), orderedKeys ...string) {
c.vLock.RLock()
defer c.vLock.RUnlock()
c.hotReloadableConfigLock.Lock()
defer c.hotReloadableConfigLock.Unlock()
configVar := configValue{
value: ptr,
defaultValue: defaultValue,
keys: orderedKeys,
}

if isHotReloadable {
c.hotReloadableConfigLock.Lock()
c.appendVarToConfigMaps(orderedKeys, &configVar)
c.hotReloadableConfigLock.Unlock()
}

for _, key := range orderedKeys {
c.bindEnv(key)
if c.isSetInternal(key) {
if c.IsSet(key) {
store(c.GetBool(key, defaultValue))
return
}
Expand Down Expand Up @@ -257,10 +253,6 @@ func (c *Config) GetReloadableFloat64Var(defaultValue float64, orderedKeys ...st
func (c *Config) registerFloat64Var(
defaultValue float64, ptr any, isHotReloadable bool, store func(float64), orderedKeys ...string,
) {
c.vLock.RLock()
defer c.vLock.RUnlock()
c.hotReloadableConfigLock.Lock()
defer c.hotReloadableConfigLock.Unlock()
configVar := configValue{
value: ptr,
multiplier: 1.0,
Expand All @@ -269,12 +261,14 @@ func (c *Config) registerFloat64Var(
}

if isHotReloadable {
c.hotReloadableConfigLock.Lock()
c.appendVarToConfigMaps(orderedKeys, &configVar)
c.hotReloadableConfigLock.Unlock()
}

for _, key := range orderedKeys {
c.bindEnv(key)
if c.isSetInternal(key) {
if c.IsSet(key) {
store(c.GetFloat64(key, defaultValue))
return
}
Expand Down Expand Up @@ -351,10 +345,6 @@ func (c *Config) GetReloadableInt64Var(defaultValue, valueScale int64, orderedKe
func (c *Config) registerInt64Var(
defaultValue int64, ptr any, isHotReloadable bool, valueScale int64, store func(int64), orderedKeys ...string,
) {
c.vLock.RLock()
defer c.vLock.RUnlock()
c.hotReloadableConfigLock.Lock()
defer c.hotReloadableConfigLock.Unlock()
configVar := configValue{
value: ptr,
multiplier: valueScale,
Expand All @@ -363,12 +353,14 @@ func (c *Config) registerInt64Var(
}

if isHotReloadable {
c.hotReloadableConfigLock.Lock()
c.appendVarToConfigMaps(orderedKeys, &configVar)
c.hotReloadableConfigLock.Unlock()
}

for _, key := range orderedKeys {
c.bindEnv(key)
if c.isSetInternal(key) {
if c.IsSet(key) {
store(c.GetInt64(key, defaultValue) * valueScale)
return
}
Expand Down Expand Up @@ -455,10 +447,6 @@ func (c *Config) registerDurationVar(
defaultValueInTimescaleUnits int64, ptr any, isHotReloadable bool, timeScale time.Duration,
store func(time.Duration), orderedKeys ...string,
) {
c.vLock.RLock()
defer c.vLock.RUnlock()
c.hotReloadableConfigLock.Lock()
defer c.hotReloadableConfigLock.Unlock()
configVar := configValue{
value: ptr,
multiplier: timeScale,
Expand All @@ -467,11 +455,13 @@ func (c *Config) registerDurationVar(
}

if isHotReloadable {
c.hotReloadableConfigLock.Lock()
c.appendVarToConfigMaps(orderedKeys, &configVar)
c.hotReloadableConfigLock.Unlock()
}

for _, key := range orderedKeys {
if c.isSetInternal(key) {
if c.IsSet(key) {
store(c.GetDuration(key, defaultValueInTimescaleUnits, timeScale))
return
}
Expand Down Expand Up @@ -548,22 +538,20 @@ func (c *Config) GetReloadableStringVar(defaultValue string, orderedKeys ...stri
func (c *Config) registerStringVar(
defaultValue string, ptr any, isHotReloadable bool, store func(string), orderedKeys ...string,
) {
c.vLock.RLock()
defer c.vLock.RUnlock()
c.hotReloadableConfigLock.Lock()
defer c.hotReloadableConfigLock.Unlock()
configVar := configValue{
value: ptr,
defaultValue: defaultValue,
keys: orderedKeys,
}

if isHotReloadable {
c.hotReloadableConfigLock.Lock()
c.appendVarToConfigMaps(orderedKeys, &configVar)
c.hotReloadableConfigLock.Unlock()
}

for _, key := range orderedKeys {
if c.isSetInternal(key) {
if c.IsSet(key) {
store(c.GetString(key, defaultValue))
return
}
Expand Down Expand Up @@ -640,22 +628,20 @@ func (c *Config) GetReloadableStringSliceVar(defaultValue []string, orderedKeys
func (c *Config) registerStringSliceVar(
defaultValue []string, ptr any, isHotReloadable bool, store func([]string), orderedKeys ...string,
) {
c.vLock.RLock()
defer c.vLock.RUnlock()
c.hotReloadableConfigLock.Lock()
defer c.hotReloadableConfigLock.Unlock()
configVar := configValue{
value: ptr,
defaultValue: defaultValue,
keys: orderedKeys,
}

if isHotReloadable {
c.hotReloadableConfigLock.Lock()
c.appendVarToConfigMaps(orderedKeys, &configVar)
c.hotReloadableConfigLock.Unlock()
}

for _, key := range orderedKeys {
if c.isSetInternal(key) {
if c.IsSet(key) {
store(c.GetStringSlice(key, defaultValue))
return
}
Expand Down Expand Up @@ -740,22 +726,20 @@ func (c *Config) GetReloadableStringMapVar(
func (c *Config) registerStringMapVar(
defaultValue map[string]interface{}, ptr any, isHotReloadable bool, store func(map[string]interface{}), orderedKeys ...string,
) {
c.vLock.RLock()
defer c.vLock.RUnlock()
c.hotReloadableConfigLock.Lock()
defer c.hotReloadableConfigLock.Unlock()
configVar := configValue{
value: ptr,
defaultValue: defaultValue,
keys: orderedKeys,
}

if isHotReloadable {
c.hotReloadableConfigLock.Lock()
c.appendVarToConfigMaps(orderedKeys, &configVar)
c.hotReloadableConfigLock.Unlock()
}

for _, key := range orderedKeys {
if c.isSetInternal(key) {
if c.IsSet(key) {
store(c.GetStringMap(key, defaultValue))
return
}
Expand Down
18 changes: 8 additions & 10 deletions config/load.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ func (c *Config) onConfigChange() {
fmt.Println(err)
}
}()
c.vLock.RLock()
defer c.vLock.RUnlock()
c.hotReloadableConfigLock.RLock()
defer c.hotReloadableConfigLock.RUnlock()
c.checkAndHotReloadConfig(c.hotReloadableConfig)
Expand All @@ -64,7 +62,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value int
var isSet bool
for _, key := range configVal.keys {
if c.isSetInternal(key) {
if c.IsSet(key) {
isSet = true
_value = c.GetInt(key, configVal.defaultValue.(int))
break
Expand All @@ -79,7 +77,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value int64
var isSet bool
for _, key := range configVal.keys {
if c.isSetInternal(key) {
if c.IsSet(key) {
isSet = true
_value = c.GetInt64(key, configVal.defaultValue.(int64))
break
Expand All @@ -94,7 +92,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value string
var isSet bool
for _, key := range configVal.keys {
if c.isSetInternal(key) {
if c.IsSet(key) {
isSet = true
_value = c.GetString(key, configVal.defaultValue.(string))
break
Expand All @@ -108,7 +106,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value time.Duration
var isSet bool
for _, key := range configVal.keys {
if c.isSetInternal(key) {
if c.IsSet(key) {
isSet = true
_value = c.GetDuration(key, configVal.defaultValue.(int64), configVal.multiplier.(time.Duration))
break
Expand All @@ -122,7 +120,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value bool
var isSet bool
for _, key := range configVal.keys {
if c.isSetInternal(key) {
if c.IsSet(key) {
isSet = true
_value = c.GetBool(key, configVal.defaultValue.(bool))
break
Expand All @@ -136,7 +134,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value float64
var isSet bool
for _, key := range configVal.keys {
if c.isSetInternal(key) {
if c.IsSet(key) {
isSet = true
_value = c.GetFloat64(key, configVal.defaultValue.(float64))
break
Expand All @@ -151,7 +149,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value []string
var isSet bool
for _, key := range configVal.keys {
if c.isSetInternal(key) {
if c.IsSet(key) {
isSet = true
_value = c.GetStringSlice(key, configVal.defaultValue.([]string))
break
Expand All @@ -167,7 +165,7 @@ func (c *Config) checkAndHotReloadConfig(configMap map[string][]*configValue) {
var _value map[string]interface{}
var isSet bool
for _, key := range configVal.keys {
if c.isSetInternal(key) {
if c.IsSet(key) {
isSet = true
_value = c.GetStringMap(key, configVal.defaultValue.(map[string]interface{}))
break
Expand Down

0 comments on commit 75af0a6

Please sign in to comment.