Remove arc and refcell

[RoDmitry]

Description:

I've looked at actix-identity middleware and I think that arc and refcell is not needed. I might be wrong, I'm still learning, but all tests have passed. And what the attache was for? Do you use it elsewhere?

Checklist:

• [ ] Tests for the changes have been added (for bug fixes / features);
• [ ] Docs have been added / updated (for bug fixes / features).
• [ ] This PR has been added to CHANGELOG.md (to [Unreleased] section);

[DDtKey]

I'm afraid we can't rely on Rc since the extractor can be used across multiple threads with parallel requests(and there may be a custom implementation of users, not only functions auto-implemented trait PermissionsExtractor).

We clone it with every transformation and request. It may be unsafe if the counter goes down to zero non-atomic (and resources will be freed).

Rather, we should think about tests that will cover this 🤔

[RoDmitry]

I've checked all of the actix-web/src/middleware/, and all of them are Rc, that's strange 🤔

[DDtKey]

I've checked all of the actix-web/src/middleware/, and all of them are Rc, that's strange 🤔

It's good point, I want to make sure as it really confuses me

[RoDmitry]

resources will be freed I think these are 'static, and can't be freed. It's even stated here: https://github.com/DDtKey/actix-web-grants/blob/bd934757fd6d75a8e448bc6733af418d1d70617b/src/middleware.rs#L86

[DDtKey]

resources will be freed I think these are 'static, and can't be freed It's even stated here:

https://github.com/DDtKey/actix-web-grants/blob/bd934757fd6d75a8e448bc6733af418d1d70617b/src/middleware.rs#L86

yeah, but 'static it is not necessarily a static lifetime, it's can be owned value also. And it can be dynamically dropped at runtime 🤔 You can see it in this article

I'll research this point a little later and will return with an answer.

[DDtKey]

@RoDmitry The reason is simpler, single-threaded workers are used in actix-web. That is, the state is not shared between threads, but a new one is created for each one. You can chek this in the actix server doc (Multi-threading section)

So, your suggestion will work well! 👏
But what do you think about make this for 3+ version (main branch)? We will merge it to v4 anyway

[RoDmitry]

Cool! So I just make same on main, instead of v4, right?

[DDtKey]

Cool! So I just make same on main, instead of v4, right?

Yep! 🙂

[RoDmitry]

Implementation for v4 is a bit different, still needs Refcell on v3.

[DDtKey]

Thank you for your contribution! 🚀