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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made AsyncOperation accessors are thread-safe and fixed project tests #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vasilenkoigor
Copy link
Contributor

Hi, guys!

  • I suggest this pull-request based on Apple's documentation, which says:

When you subclass NSOperation, you must make sure that any overridden methods remain safe to call from multiple threads. If you implement custom methods in your subclass, such as custom data accessors, you must also make sure those methods are thread-safe. Thus, access to any data variables in the operation must be synchronized to prevent potential data corruption.
NSOperation docs

Analyzing the current code, you can understand that the accessors of our subclass are not fully thread safe.

  • Also, I fixed compiler errors tests building and 'testThatCompoundOperationCancellsCorrectly' fails

I hope this will improve the quality of the project.

@novixon
Copy link
Collaborator

novixon commented May 21, 2017

Hi, @vasilenkoigor!

Thanks for your pull request. The idea to fix the probable issues of asynchronous work is very nice. But before fixing them, we should try to find them out at first. Could you, please, write tests that would show the race conditions and other multithreading issues in the old codebase?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants