Navigation Menu

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

Fix a race condition with the initialization of class metadata #22409

Merged

Conversation

rjmccall
Copy link
Member

@rjmccall rjmccall commented Feb 6, 2019

In our initial approach for resolving metadata dependency cycles with classes, non-transitively complete superclass metadata was fetched by the subclass's metadata completion function and passed to swift_initClassMetadata. That could mean generating quite a lot of code in the completion function, and so we fairly recently changed it so that swift_initClassMetadata instead fetched the superclass metadata via a demangling. Unfortunately, the metadata demangler only fetches abstract metadata by default, and class metadata cannot be considered even non-transitively complete when its superclass reference not at that stage. If the superclass metadata is being completed on one thread, and a subclass is being completed on another, and the subclass installs the incomplete superclass metadata in its superclass field and attempts to register the subclass with the Objective-C runtime, the runtime may crash reading the incompletely-initialized superclass.

The proper fix is to make swift_initClassMetadata fetch non-transitively complete metadata for the superclass, delaying completion if that metadata is unavailable. Unfortunately, that can't actually be implemented on top of swift_initClassMetadata because that function has no means of reporting an unsatisfied dependency to its caller, and we can no longer simply change its signature without worrying about a small of internal code that might still be using it. We cannot simply perform a blocking metadata request in swift_initClassMetadata because it is deeply problematic to block within a metadata completion function. The solution is therefore to add a swift_initClassMetadata2 which has the ability to report unsatisfied dependencies. That was done in #22386; this patch builds on that by teaching the compiler to generate code to actually use it. It is therefore not safe to use this patch if you might be running on an OS that only provides the old runtime function, but that should be a temporary Apple-internal problem.

Fixes rdar://47549859.

This PR includes a small fix to #22386 which has already been incorporated into #22400, the 5.0 version of that PR. It also addresses some review feedback left on #22386.

Somehow this only broke tests on the 5.0 branch.
In our initial approach for resolving metadata dependency cycles with classes, non-transitively complete superclass metadata was fetched by the subclass's metadata completion function and passed to `swift_initClassMetadata`. That could mean generating quite a lot of code in the completion function, and so we fairly recently changed it so that `swift_initClassMetadata` instead fetched the superclass metadata via a demangling. Unfortunately, the metadata demangler only fetches _abstract_ metadata by default, and class metadata cannot be considered even non-transitively complete when its superclass reference not at that stage.  If the superclass metadata is being completed on one thread, and a subclass is being completed on another, and the subclass installs the incomplete superclass metadata in its superclass field and attempts to register the subclass with the Objective-C runtime, the runtime may crash reading the incompletely-initialized superclass.

The proper fix is to make `swift_initClassMetadata` fetch non-transitively complete metadata for the superclass, delaying completion if that metadata is unavailable. Unfortunately, that can't actually be implemented on top of `swift_initClassMetadata` because that function has no means of reporting an unsatisfied dependency to its caller, and we can no longer simply change its signature without worrying about a small of internal code that might still be using it. We cannot simply perform a blocking metadata request in `swift_initClassMetadata` because it is deeply problematic to block within a metadata completion function. The solution is therefore to add a `swift_initClassMetadata2` which has the ability to report unsatisfied dependencies. That was done in apple#22386; this patch builds on that by teaching the compiler to generate code to actually use it. It is therefore not safe to use this patch if you might be running on an OS that only provides the old runtime function, but that should be a temporary Apple-internal problem.

Fixes rdar://47549859.
@rjmccall
Copy link
Member Author

rjmccall commented Feb 6, 2019

@swift-ci Please test.

@swift-ci
Copy link
Collaborator

swift-ci commented Feb 6, 2019

Build failed
Swift Test OS X Platform
Git Sha - 79d1581

@rjmccall
Copy link
Member Author

rjmccall commented Feb 6, 2019

...I don't think this IRGen-only patch caused a SILOptimizer failure on the i386 iPhone simulator.

@rjmccall
Copy link
Member Author

rjmccall commented Feb 7, 2019

@swift-ci Please test.

@rjmccall rjmccall merged commit f143db7 into apple:master Feb 7, 2019
@rjmccall rjmccall deleted the init-class-metadata-dependencies-compiler branch February 7, 2019 18:22
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