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

Use MutexLocker when ASImageNode traits change #1662

Merged
merged 1 commit into from
Sep 6, 2019
Merged

Conversation

bolsinga
Copy link
Contributor

@bolsinga bolsinga commented Sep 6, 2019

Then access the ivars directly so that another lock access is not required. Using this C++ macro ensures that any future early returns or the like will still unlock.

…o that another lock access is not required
Copy link
Contributor

@mikezucc mikezucc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may actually fix the crash we were trying to address. The closest I ever got to the crashing stack was to call dealloc on tc

@bolsinga bolsinga changed the title Use MutexLocker for safe locking and then access the ivars directly s… Use MutexLocker for safe locking Sep 6, 2019
@vovasty
Copy link
Contributor

vovasty commented Sep 6, 2019

I vote for accessing imageAsset from main thread.

@bolsinga
Copy link
Contributor Author

bolsinga commented Sep 6, 2019

I vote for accessing imageAsset from main thread.

I also think that is a good follow up PR.

@bolsinga bolsinga changed the title Use MutexLocker for safe locking Use MutexLocker for safe locking when ASImageNode traits change Sep 6, 2019
@bolsinga bolsinga changed the title Use MutexLocker for safe locking when ASImageNode traits change Use MutexLocker when ASImageNode traits change Sep 6, 2019
@bolsinga bolsinga merged commit a403160 into master Sep 6, 2019
@bolsinga bolsinga deleted the TraitChangeLocking branch September 6, 2019 20:02
@bolsinga
Copy link
Contributor Author

bolsinga commented Sep 6, 2019

I vote for accessing imageAsset from main thread.

I also think that is a good follow up PR.

And that looks like #1663

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

Successfully merging this pull request may close these issues.

4 participants