Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix thread-safety issue for serialization queue usage. #2885

Merged
merged 2 commits into from Jul 31, 2019

Conversation

@jshier
Copy link
Contributor

commented Jul 31, 2019

Issue Link 馃敆

During development of another feature I encountered a thread sanitizer issue during testing. Turns out that our response serializer queue handling wasn't correct and was modifying Request state from the serializer queue instead of the root queue.

Goals 鈿斤笍

This PR fixes the issue by dispatching back to the root queue for parts of the serialization that don't need to be on the serialization queue.

Implementation Details 馃毀

This PR clarifies which parts of serializer calls will be called on the serialization queue and which will happen on the root queue.

Testing Details 馃攳

Added an additional test using the request and serialization queues, but to really test this we need some sort of stress tests.

@cnoon cnoon self-requested a review Jul 31, 2019

@cnoon
cnoon approved these changes Jul 31, 2019
Copy link
Member

left a comment

Looks good to me @jshier, good catch.

@jshier jshier merged commit 183d44f into master Jul 31, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jshier jshier deleted the bugfix/response-serializer-threadsafety branch Jul 31, 2019

@jshier jshier added this to the 5.0.0.rc.1 milestone Aug 6, 2019

toomasr added a commit to toomasr/Alamofire that referenced this pull request Aug 19, 2019
Fix thread-safety issue for serialization queue usage. (Alamofire#2885)
* Fix thread-safety issue for serialization queue usage.

* Add launch screen for example.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.