Thread safety about casbin HOT 11 CLOSED

Thanks for pointing this out. My initial plan is using a RWMutex, Enforcer.Enforce() will be protected with read lock and functions like Enforcer.AddRoleForUser() will be protected with write lock, what do you think?

from casbin.

For my use case this is workable. If anyone else has performance concerns and does not have the use case of dynamic policies etc. this could be made as a thin wrapper.

from casbin.

I have added a wrapper here: 9251d5d

from casbin.

This looks great and it dropped in without issues, after a bit of testing I'll go ahead and close this thread. Thanks!

from casbin.

The Safe variants of methods (ones that return an error instead of panic) aren't thread safe at the moment because they are not wrapped.

from casbin.

@e-nikolov , we have "Safe" methods and "Synced" enforcer. These are two different things. "Safe" methods return error instead of causing panic. "Synced" enforcer will provide thread safe. You should use this: https://github.com/casbin/casbin/blob/master/enforcer_synced.go

from casbin.

@hsluoyz I know that, but the safe methods are also available from the synced enforcer, however they are not thread safe because they are not wrapped with mutexes the way the non-safe methods are.

from casbin.

OK. So you need both safe methods and synced enforcer at the same time, right? We can add the enforcer_safe_synced.go file to implement those functions.

Can you send a PR for it? I can also do it but it may take several days.

from casbin.

BTW, you can also propose a new issue to claim this problem, as this issue has already been closed for long time.

from casbin.

I could make a PR. I am only thinking that in the current model, there are enforcer, enforcer_synced, enforcer_safe, enforcer_cached. So every time that a new combination of those is needed, methods would sort of have to be duplicated. Is it an option to use something like Functional Options, and be able to do this: casbin.NewEnforcer(casbin.Synced(), casbin.Cached())?

from casbin.

casbin.NewEnforcer(casbin.Synced(), casbin.Cached()) will make the constructor even complicated. Do we have a non-intrusive way to specify synced or cached? e.g., enforcer.enableCache(), enforcer.enableSync() ?

from casbin.