fix: lock less when syncing

enforcer.go

@@ -281,46 +281,37 @@ func (e *Enforcer) ClearPolicy() {
 
 // LoadPolicy reloads the policy from file/database.
 func (e *Enforcer) LoadPolicy() error {
-	needToRebuild := false
-	newModel := e.model.Copy()
-	newModel.ClearPolicy()
+	newModel, err := e.prepareNewModel(e.model.Copy())
+	if err != nil {
+		return err
+	}
 
-	var err error
-	defer func() {
-		if err != nil {
-			if e.autoBuildRoleLinks && needToRebuild {
-				_ = e.BuildRoleLinks()
-			}
+	e.model = newModel
+	if e.autoBuildRoleLinks {
+		if err := e.BuildRoleLinks(); err != nil {
+			return err
 		}
-	}()
-
-	if err = e.adapter.LoadPolicy(newModel); err != nil && err.Error() != "invalid file path, file path cannot be empty" {
-		return err
 	}
+	return nil
+}
 
-	if err = newModel.SortPoliciesBySubjectHierarchy(); err != nil {
-		return err
+// prepareNewModel prepares a new model with the policy from file/database.
+func (e *Enforcer) prepareNewModel(newModel model.Model) (model.Model, error) {
+	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.SortPoliciesByPriority(); err != nil {
-		return err
+	if err := newModel.SortPoliciesBySubjectHierarchy(); err != nil {
+		return nil, err
 	}
 
-	if e.autoBuildRoleLinks {
-		needToRebuild = true
-		for _, rm := range e.rmMap {
-			err := rm.Clear()
-			if err != nil {
-				return err
-			}
-		}
-		err = newModel.BuildRoleLinks(e.rmMap)
-		if err != nil {
-			return err
-		}
+	if err := newModel.SortPoliciesByPriority(); err != nil {
+		return nil, err
 	}
-	e.model = newModel
-	return nil
+
+	return newModel, nil
 }
 
 func (e *Enforcer) loadFilteredPolicy(filter interface{}) error {

enforcer_synced.go

@@ -111,9 +111,23 @@ func (e *SyncedEnforcer) ClearPolicy() {
 
 // LoadPolicy reloads the policy from file/database.
 func (e *SyncedEnforcer) LoadPolicy() error {
+	e.m.RLock()
+	cleanedNewModel := e.model.Copy()
+	e.m.RUnlock()
+	newModel, err := e.prepareNewModel(cleanedNewModel)
+	if err != nil {
+		return err
+	}
+
 	e.m.Lock()
 	defer e.m.Unlock()
-	return e.Enforcer.LoadPolicy()
+	e.model = newModel
+	if e.autoBuildRoleLinks {
+		if err := e.Enforcer.BuildRoleLinks(); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 // LoadFilteredPolicy reloads a filtered policy from file/database.

enforcer_synced_test.go

@@ -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()
 }

go.mod

@@ -5,4 +5,4 @@ require (
 	github.com/golang/mock v1.4.4
 )
 
-go 1.13
+go 1.13