search

casbin-rs / actix-casbin-auth

Casbin Actix-web access control middleware
https://github.com/casbin/casbin-rs
55 stars 15 forks source link

maybe `enforce_mut()` should be used? #16

Closed sify21 closed 4 years ago

sify21 commented 4 years ago

An enforcer is put in an Arc<RwLock<>> and is write-locked before use. But enforce() method takes an immutable reference, it doesn't need to be locked. Maybe enforce_mut should be used? https://github.com/casbin/casbin-rs/blob/c88f13cb59a428b05a4f9b03e83f3f5bea36dd78/src/cached_enforcer.rs#L233

issue-label-bot[bot] commented 4 years ago

Issue-Label Bot is automatically applying the label feature_request to this issue, with a confidence of 0.62. Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

GopherJ commented 4 years ago

@hackerchai this is urgent, could you do it?

hackerchai commented 4 years ago

@hackerchai this is urgent, could you do it?

@GopherJ I will have a try.

hackerchai commented 4 years ago

An enforcer is put in an Arc<RwLock<>> and is write-locked before use. But enforce() method takes an immutable reference, it doesn't need to be locked. Maybe enforce_mut should be used? https://github.com/casbin/casbin-rs/blob/c88f13cb59a428b05a4f9b03e83f3f5bea36dd78/src/cached_enforcer.rs#L233

@sify21 Thanks for the advise. But unfortunately, both Enforcer and CachedEnforcer are not sync at present. When we are using actix middleware, we have to do modification to the CachedEnforcer and the middleware is multi-threading. So we can't wrap the CachedEnforcer with Arc. For now, we can only use Arc<Mutex<CachedEnforcer>> or Arc<RwLock<CachedEnforcer>> to ensure the safety. In future, we plan to implement the sync enforcer to avoid the usage of lock.

sify21 commented 4 years ago

https://github.com/casbin-rs/actix-casbin-auth/commit/c6f045205c14a6abf3d81b6c9693966c1c29568b should fix this issue. I'll close