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

[ASTextNode] Fix a deadlock that could occur when enabling experimental ASTextNode2 via ASConfiguration #903

Merged
merged 1 commit into from
May 3, 2018

Conversation

appleguy
Copy link
Member

@appleguy appleguy commented May 3, 2018

Because multiple threads can enter this allocWithZone: method around the same time, it is possible for one of them to setSuperclass first, and then the second thread would get stuck in an infinite loop.

When climbing the inheritance hierarchy, ASTextNode2 would be encountered by this second thread, and it would continue until reaching c == Nil. Since there was no case to catch this, an infinite loop would result.

Then the main thread can be deadlocked if a method like waitUntilAllUpdatesAreProcessed is called on ASCollectionView.

…al ASTextNode2 via ASConfiguration

Because multiple threads can enter this allocWithZone: method around the same time, it is possible for one of them to setSuperclass first, and then the second thread would get stuck in an infinite loop. When climbing the inheritance heirarchy, ASTextNode2 would be encountered by this second thread, and it would continue until reaching c == Nil. Since there was no case to catch this, an infinite loop would result. Then the main thread can be deadlocked if a method like waitUntilAllUpdatesAreProcessed is called on ASCollectionView.
@ghost
Copy link

ghost commented May 3, 2018

🚫 CI failed with log

@appleguy
Copy link
Member Author

appleguy commented May 3, 2018

Failure looks spurious - any way to re-run? Tests pass locally.

@ghost
Copy link

ghost commented May 3, 2018

1 Warning
⚠️ Any source code changes should have an entry in CHANGELOG.md or have #trivial in their title.

Generated by 🚫 Danger

Copy link
Member

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

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

Good catch!

@Adlai-Holler Adlai-Holler merged commit 9de3336 into master May 3, 2018
@Adlai-Holler Adlai-Holler deleted the ASTextNode2InfLoop branch May 3, 2018 10:52
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.

None yet

2 participants