Perform an Extra Consistency Check When Searching The ModuleManager's Cache For Implicit Modules #1718
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 7fb8148.
Author
|
@swift-ci test |
DougGregor
approved these changes
Aug 28, 2020
The ModuleManager's use of FileEntry nodes as the keys for its map of loaded modules is less than ideal. Uniqueness for FileEntry nodes is maintained by FileManager, which in turn uses inode numbers on hosts that support that. When coupled with the module cache's proclivity for turning over and deleting stale PCMs, this means entries for different module files can wind up reusing the same underlying inode. When this happens, subsequent accesses to the Modules map will disagree on the ModuleFile associated with a given file. In general, it is not sufficient to resolve this conundrum with a type like FileEntryRef that stores the name of the FileEntry node on first access because of path canonicalization issues. However, the paths constructed for implicit module builds are fully under Clang's control. We *can*, therefore, rely on their structure being consistent across operating systems and across subsequent accesses to the Modules map. To mitigate the effects of inode reuse, perform an extra name check when implicit modules are returned from the cache. This has the effect of forcing reused FileEntry nodes to stomp over existing-but-stale entries in the cache, which simulates a miss - exactly the desired behavior. rdar://48443680
Author
|
@swift-ci test |
Author
|
@swift-ci test Linux |
1 similar comment
Author
|
@swift-ci test Linux |
This was referenced Aug 29, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts out the failed approach in #1651 which did nothing to address the symptoms of inode reuse. The patch on top of this provides an alternative approach: For implicit module builds check the name that we expect an entry to be filed under in the Modules cache actually matches the one we're asked to look for. Note that we can only do this for implicit module builds because the paths that these files are looked up under are fully under the control of Clang. For explicit module builds, this consistency check will fail on OSes that use different path forms than the *NIXes (e.g. Windows) because of canonicalization issues.
rdar://48443680