[Question] concurrent race when LoadPolicy is called
@hsluoyz is already working on this.
Since Oct 11, 2022.
Assessment
This issue has not been assessed yet.
Description
Want to prioritize this issue? Try:
What's your scenario? What do you want to achieve?
Sometimes when a watcher calls LoadPolicy to synchronize policies, panic occurs:
fatal error: concurrent map iteration and map write
goroutine 152 [running]:
runtime.throw({0x2557e5c?, 0x3711778?})
/usr/local/Cellar/go/1.18.3/libexec/src/runtime/panic.go:992 +0x71 fp=0xc000c9b818 sp=0xc000c9b7e8 pc=0x103e7d1
runtime.mapiternext(0xc000307dd0?)
/usr/local/Cellar/go/1.18.3/libexec/src/runtime/map.go:871 +0x60a fp=0xc000c9b8a8 sp=0xc000c9b818 pc=0x1012eea
runtime.mapiterinit(0x234dae0?, 0xc00064d110, 0xc000c9ba78)
/usr/local/Cellar/go/1.18.3/libexec/src/runtime/map.go:861 +0x18c fp=0xc000c9b8c8 sp=0xc000c9b8a8 pc=0x101288c
github.com/casbin/casbin/v2/model.(*Assertion).copy(0xc0008fe880)
/Users/sasakiyori/go/pkg/mod/github.com/casbin/casbin/v2@v2.47.2/model/assertion.go:108 +0x2cf fp=0xc000c9bae8 sp=0xc000c9b8c8 pc=0x199c00f
github.com/casbin/casbin/v2/model.Model.Copy(0xc0006dd800)
/Users/sasakiyori/go/pkg/mod/github.com/casbin/casbin/v2@v2.47.2/model/model.go:384 +0x1b0 fp=0xc000c9bc58 sp=0xc000c9bae8 pc=0x19a0cd0
github.com/casbin/casbin/v2.(*Enforcer).LoadPolicy(0xc000684000)
/Users/sasakiyori/go/pkg/mod/github.com/casbin/casbin/v2@v2.47.2/enforcer.go:291 +0x5a fp=0xc000c9bdc8 sp=0xc000c9bc58 pc=0x19b76ba
github.com/casbin/casbin/v2.(*Enforcer).SetWatcher.func1({0x1c29, 0xa})
/Users/sasakiyori/go/pkg/mod/github.com/casbin/casbin/v2@v2.47.2/enforcer.go:260 +0x25 fp=0xc000c9bde8 sp=0xc000c9bdc8 pc=0x19b7645
github.com/casbin/etcd-watcher/v2.(*Watcher).startWatch(0xc000349c00)
/Users/sasakiyori/go/pkg/mod/github.com/casbin/etcd-watcher/v2@v2.1.1/watcher.go:127 +0x268 fp=0xc000c9bfc0 sp=0xc000c9bde8 pc=0x2289c28
github.com/casbin/etcd-watcher/v2.NewWatcher.func1()
/Users/sasakiyori/go/pkg/mod/github.com/casbin/etcd-watcher/v2@v2.1.1/watcher.go:67 +0x25 fp=0xc000c9bfe0 sp=0xc000c9bfc0 pc=0x22892e5
runtime.goexit()
/usr/local/Cellar/go/1.18.3/libexec/src/runtime/asm_amd64.s:1571 +0x1 fp=0xc000c9bfe8 sp=0xc000c9bfe0 pc=0x1072e01
created by github.com/casbin/etcd-watcher/v2.NewWatcher
/Users/sasakiyori/go/pkg/mod/github.com/casbin/etcd-watcher/v2@v2.1.1/watcher.go:66 +0x2a8
I can't reproduce it every time, but I think the problem may arised here:
As the call chain shown below, when a goroutine do iteration in ast.PolicyMap from ast.copy(), any other goroutines write into this map(such as AddPolicy) may cause panic.
func (e *Enforcer) LoadPolicy() error {
needToRebuild := false
// panic occurs
newModel := e.model.Copy()
newModel.ClearPolicy()
// ...
}
func (model Model) Copy() Model {
newModel := NewModel()
for sec, m := range model {
newAstMap := make(AssertionMap)
// ast is a pointer of AssertionMap
for ptype, ast := range m {
newAstMap[ptype] = ast.copy()
}
newModel[sec] = newAstMap
}
newModel.SetLogger(model.GetLogger())
return newModel
}
func (ast *Assertion) copy() *Assertion {
tokens := append([]string(nil), ast.Tokens...)
policy := make([][]string, len(ast.Policy))
for i, p := range ast.Policy {
policy[i] = append(policy[i], p...)
}
policyMap := make(map[string]int)
// map is unsafe between goroutines
// when it is iterating, other goroutine's write operation may cause panic
for k, v := range ast.PolicyMap {
policyMap[k] = v
}
newAst := &Assertion{
Key: ast.Key,
Value: ast.Value,
PolicyMap: policyMap,
Tokens: tokens,
Policy: policy,
FieldIndexMap: ast.FieldIndexMap,
}
return newAst
}
And What confused me is:
After copy a new model, it will clear policy map, so the copy of PolicyMap from ast.copy() is useless.
Could you create a new copy function to eliminate these unsafe operations?
func (e *Enforcer) LoadPolicy() error {
needToRebuild := false
newModel := e.model.Copy()
newModel.ClearPolicy()
// ...
}
func (model Model) ClearPolicy() {
for _, ast := range model["p"] {
ast.Policy = nil
ast.PolicyMap = map[string]int{}
}
for _, ast := range model["g"] {
ast.Policy = nil
ast.PolicyMap = map[string]int{}
}
}
- Dominant language
- Go
- Stars
- 20.4k
- Forks
- 1.8k
- Avg merge
- 5d 11h
- Merged PRs (30d)
- 2
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from apache/casbin
-
question
-
enhancement
-
enhancement
-
enhancement
-
enhancement
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
Difficulty 1/5 Under an hour Newbie friendliness 84/100
-
enhancement needs triage
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
kind/cleanup
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
kubernetes-sigs/kueue#15947 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
sympozium-ai/sympozium#627 ·