fix: revert lock less when syncing

[johanneswuerbach]

Reverts #988

This change can cause races when the management api is used while a new model is being prepared.

Situation:

• T1: Routine 1: loads new model from db into newModel
• T2: Routine 2: write change against db and current model
• T3: Routine 1: replace model with newModel

In this case the write in T2 would not be present until the next reload.

Sadly that brings back the issue that slow policy loading stalls any enforcements. One option might be to use a different lock for such performance critical operation and accept a certain degree of eventual consistency.

[casbin-bot]

@tangyang9464 @closetool @sagilio please review

[hsluoyz]

@johanneswuerbach plz fix:

[johanneswuerbach]

@hsluoyz fix pushed.

[tangyang9464]

@johanneswuerbach Maybe we don't need to revert, just add a read lock to LoadPolicy from the beginning. Temporary inconsistencies are allowed, read but not write.

[johanneswuerbach]

That won’t work as an open read lock blocks any writes and there is no way to upgrade a read to write lock in RWMutex today golang/go#38859 (comment)

[tangyang9464]

@johanneswuerbach I don't understand very well, isn't our goal to add RLock, only allow reading and not writing? My code is

func (e *SyncedEnforcer) LoadPolicy() error {
	e.m.RLock()
	defer e.m.RUnlock()
	return e.Enforcer.LoadPolicy()
}

[johanneswuerbach]

@tangyang9464 sorry I misunderstood your proposal. LoadPolicy is doing changes on the enforcer (e.g.

casbin/enforcer.go

Line 440 in 290ed05

for _, rm := range e.rmMap {

) and I'm not sure what happens if reads occur during those changes.

[tangyang9464]

@johanneswuerbach yes, we need use new rmMap like newModel. But role manager has no Copy interface. Maybe we need to add it for this.

[johanneswuerbach]

Hey @tangyang9464, my company is likely migrating away from casbin due to several scalability issues discovered with our usage and the intransparent maintainer decision making, so I won't have much time to implement those changes.

I just wanted to revert this change, as I've originally introduced it in #1026.

[github-actions]

🎉 This PR is included in version 2.47.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀