search

GetStream / stream-swift

Swift client for Stream API
https://getstream.io
BSD 3-Clause "New" or "Revised" License
35 stars 26 forks source link

Make `AuthorizationMoyaPlugin` thread-safe #44

Closed nuno-vieira closed 2 years ago

nuno-vieira commented 2 years ago

Description of the pull request

Fixes issue: https://github.com/GetStream/stream-swift/issues/40

nuno-vieira commented 2 years ago

Thanks for this! Not sure if you have testing for this but maybe adding the rest for concurrency would be advisable?

What do you mean by adding the rest for concurrency?

I've tested this manually, and it is working fine. Unfortunately, our team does not have many resources allocated to this project at the moment and currently, the tests are not running. It will require a bit of time to understand how the project is set up and to fix the tests.

Since we already run the tests with the thread sanitiser and this worked, it should be fine.

angelolloqui commented 2 years ago

Sorry, a typo. I meant adding a “test” but I understand the situation. Looking forward for this merge

nuno-vieira commented 2 years ago

Sorry, a typo. I meant adding a “test” but I understand the situation. Looking forward to this merge

No problem. 👍 I've just tested this solution with the Thread Sanitizer and it is working well, with no warnings 👍 So once it is approved we will merge it.

angelolloqui commented 2 years ago

I just wanted to follow up on this and say that so far looks promising! More than 50% my audience has now the updated GetStream lib and not a single crash from it received yet, compared to the many hundreds we get in a weekly basis with the previous version!