knadh
v2.3.0 introduced mutex locks and my custom merge function stuck
Describe the bug After upgrading to v2.3.0, I noticed that there's new mutex lock added to all the main functions. I wanted to make sure that in my project, all keys are case insensitive (as i moved from Viper hehe), I make sure all keys are lowercased.
In the custom merge function, i've added k.GetString("env") to get the environment variable so that I can have diff log level on the merge outcome.
This has caused the library to hit ko.mu.RLock() and stuck.
Technically I can just remove this, but would be bad in case anything goes wrong.
Any suggestion please?
Or is there any built in way to make all keys lowercased?
To Reproduce
Use custom merge function and try to read and value from k eg: k.String("") in it.
Expected behavior A clear and concise description of what you expected to happen. Before introduction of mutex locks it was okay.
Please provide the following information):
- OS: oxs
- Koanf Version v2.3.0
Additional context
- The "ENV" is always available and loaded first from environment.
should i just use os.Getenv("XXX_ENV") for this (i have prefix in env provider)
Sorry, I missed this. Could you please share your custom merge function to see how exactly it's triggering a mutex-block?
It looks like the problem is that merge acquired a write lock prior to calling the custom merge function. Since the custom merge function calls k.String("env"), which eventually calls Get, the Get function can't acquire the RLock.
Claude put together this test that reproduces the issue: func TestMergeFunctionDeadlock(t *testing.T) { k := koanf.New(".") k.Load(confmap.Provider(map[string]interface{}{"key": "value"}, "."), nil)
done := make(chan struct{})
go func() {
k.Load(confmap.Provider(map[string]interface{}{"new": "data"}, "."), nil,
koanf.WithMergeFunc(func(src, dest map[string]interface{}) error {
_ = k.String("key") // This causes deadlock - read lock inside write lock
return nil
}))
close(done)
}()
select {
case <-done:
t.Log("No deadlock")
case <-time.After(2 * time.Second):
t.Fatal("DEADLOCK: merge function called k.String() while holding write lock")
}
}
knadh, is this something you want the library to support? If so, I'm happy to take a pass at supporting it.
Ah, that explains it @NishThakkar. Please do send a PR addressing this isssue.
As a workaround, you can read directly from the dest map instead of calling k.String() - e.g. env, _ := dest["env"].(string). The dest parameter is the current config map, so it already has all existing values. You lose the typed getters but it avoids the deadlock.
Introducing something new like:
func WithMergeFuncKoanf(fn func(src, dest map[string]interface{}, current *Koanf) error)
could be another way to solve this, where current is injected via ko.Copy(). Along with this, a documented note mentioning that the koanf instance should not be used inside WithMergeFunc.
