Skip to content

Commit

Permalink
fix: lock less when syncing
Browse files Browse the repository at this point in the history
  • Loading branch information
johanneswuerbach committed Apr 14, 2022
1 parent 54eba5e commit 59f633e
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 14 deletions.
42 changes: 29 additions & 13 deletions enforcer.go
Expand Up @@ -281,10 +281,38 @@ func (e *Enforcer) ClearPolicy() {

// LoadPolicy reloads the policy from file/database.
func (e *Enforcer) LoadPolicy() error {
needToRebuild := false
newModel, err := e.prepareNewModel()
if err != nil {
return err
}

return e.setNewModel(newModel)
}

// prepareNewModel prepares a new model with the policy from file/database.
func (e *Enforcer) prepareNewModel() (model.Model, error) {
newModel := e.model.Copy()
newModel.ClearPolicy()

if err := e.adapter.LoadPolicy(newModel); err != nil && err.Error() != "invalid file path, file path cannot be empty" {
return nil, err
}

if err := newModel.SortPoliciesBySubjectHierarchy(); err != nil {
return nil, err
}

if err := newModel.SortPoliciesByPriority(); err != nil {
return nil, err
}

return newModel, nil
}

// setNewModel replaces the current model with a new one
func (e *Enforcer) setNewModel(newModel model.Model) error {
needToRebuild := false

var err error
defer func() {
if err != nil {
Expand All @@ -294,18 +322,6 @@ func (e *Enforcer) LoadPolicy() error {
}
}()

if err = e.adapter.LoadPolicy(newModel); err != nil && err.Error() != "invalid file path, file path cannot be empty" {
return err
}

if err = newModel.SortPoliciesBySubjectHierarchy(); err != nil {
return err
}

if err = newModel.SortPoliciesByPriority(); err != nil {
return err
}

if e.autoBuildRoleLinks {
needToRebuild = true
for _, rm := range e.rmMap {
Expand Down
14 changes: 13 additions & 1 deletion enforcer_synced.go
Expand Up @@ -69,7 +69,7 @@ func (e *SyncedEnforcer) StartAutoLoadPolicy(d time.Duration) {
select {
case <-ticker.C:
// error intentionally ignored
_ = e.LoadPolicy()
_ = e.reloadPolicy()
// Uncomment this line to see when the policy is loaded.
// log.Print("Load policy for time: ", n)
n++
Expand Down Expand Up @@ -116,6 +116,18 @@ func (e *SyncedEnforcer) LoadPolicy() error {
return e.Enforcer.LoadPolicy()
}

// reloadPolicy reloads the policy from file/database, but only locks when replacing the model
func (e *SyncedEnforcer) reloadPolicy() error {
newModel, err := e.prepareNewModel()
if err != nil {
return err
}

e.m.Lock()
defer e.m.Unlock()
return e.setNewModel(newModel)
}

// LoadFilteredPolicy reloads a filtered policy from file/database.
func (e *SyncedEnforcer) LoadFilteredPolicy(filter interface{}) error {
e.m.Lock()
Expand Down
9 changes: 9 additions & 0 deletions enforcer_synced_test.go
Expand Up @@ -40,6 +40,15 @@ func TestSync(t *testing.T) {
testEnforceSync(t, e, "bob", "data2", "read", false)
testEnforceSync(t, e, "bob", "data2", "write", true)

// Simulate a policy change
e.ClearPolicy()
testEnforceSync(t, e, "bob", "data2", "write", false)

// Wait for at least one sync
time.Sleep(time.Millisecond * 300)

testEnforceSync(t, e, "bob", "data2", "write", true)

// Stop the reloading policy periodically.
e.StopAutoLoadPolicy()
}
Expand Down

0 comments on commit 59f633e

Please sign in to comment.