Slava Pestov 85fc422dc8 Serialization: Fix weird re-entrancy issue with extension binding ...

This is like a single-threaded variant of the "lost wakeup
problem" that's all too common to anyone who's worked on
concurrent code.

When we perform lookup into a nominal type, we check if the
ASTContext's generation number is different than a cached
generation number in the nominal type. If the two numbers
differ, we walk over all loaded module files, telling them
to load any serialized extensions. Then we update the cached
generation number in the nominal type to record the fact
that we loaded any outstanding extensions.

The idea is to avoid unnecessary work if we know that no new
extensions have been added since the last name lookup.

The "bottom half" here is that when we add a new serialized
module file, we increment the ASTContext's generation number,
and then add an entry for the module file to a list.

The problem was that in between incrementing the ASTContext's
generation number and adding the module file, we would do some
work involving the ClangImporter which could in turn trigger
name lookup, which would "see" the new generation number in
the ASTContext, but not the new thing that is about to be
added, because it hasn't been added yet. So the
NominalTypeDecl's cached generation number would move forward
and the subsequent add of the module file would be "lost".

Specifically, it looks like when SerializedModuleLoader::loadAST()
calls loadedModuleFile->associateWithFileContext(), which does
some crazy ClangImporter stuff I don't understand, which in
turn can trigger a name lookup.

The fix appears to be to bump the generation number *after*
calling associateWithFileContext().

I don't completely understand what went wrong. For example,
this was dependent on the order of 'import' statements in the
input file. Of the two test cases I added, one the first one
triggered the problem -- the other test case is identical,
except the two import statements are transposed. I'm adding it
to ensure we avoid regressing in this case also.

Also I suspect it is possible to construct a test case that
does not depend on Objective-C interop or Foundation, but
again this looked tricky and I don't think the additional test
coverage on Linux would be worth the effort.

Fixes <rdar://problem/30817732>, so RxSwift now builds again on
master. Yay!

2017-03-16 22:26:25 -07:00

21 KiB

Raw Blame History

View Raw