未验证 提交 a2448b02 编写于 作者: T Tomáš Rylek 提交者: GitHub

Fix race condition in loading assemblies with composite native images (#67000)

I originally hit this bug when testing my Crossgen2 perf optimization
change involving only registering composite code range once but I
managed to disprove the bug was caused by this change. Indeed, it
was caused by my earlier change improving component assembly caching.

In my overzealous attempt to make sure the assembly gets always
registered I added a new method NativeImage::AddComponentAssemblyToCache
that registered the assembly which had triggered the native image load
in the first place. This was incorrect, by that point the assembly
wasn't yet fully initialized and putting it in the cache made it
visible to other threads that subsequently crashed trying to access
the assembly before it was fully loaded.

Furthermore it was unnecessary, once the assembly gets loaded, it
gets stored in the cache via the call to
SetNativeMetadataAssemblyRefInCache from ZapSig::DecodeModuleFromIndex.
Before the fix I was able to repro the bug locally typically after
about 500 iterations of the thread14 unit test, with the fix I was
unable to repro after 4500 iterations.

Thanks

Tomas

Fixes: https://github.com/dotnet/runtime/issues/66954
Probably also fixes: https://github.com/dotnet/runtime/issues/66210
上级 a30a2510
......@@ -126,7 +126,6 @@ NativeImage *NativeImage::Open(
*isNewNativeImage = false;
if (pExistingImage->GetAssemblyBinder() == pAssemblyBinder)
{
pExistingImage->AddComponentAssemblyToCache(componentModule->GetAssembly());
return pExistingImage;
}
else
......@@ -246,14 +245,12 @@ NativeImage *NativeImage::Open(
// No pre-existing image, new image has been stored in the map
*isNewNativeImage = true;
amTracker.SuppressRelease();
image->AddComponentAssemblyToCache(componentModule->GetAssembly());
return image.Extract();
}
// Return pre-existing image if it was loaded into the same ALC, null otherwise
*isNewNativeImage = false;
if (pExistingImage->GetAssemblyBinder() == pAssemblyBinder)
{
pExistingImage->AddComponentAssemblyToCache(componentModule->GetAssembly());
return pExistingImage;
}
else
......@@ -263,19 +260,6 @@ NativeImage *NativeImage::Open(
}
#endif
#ifndef DACCESS_COMPILE
void NativeImage::AddComponentAssemblyToCache(Assembly *assembly)
{
STANDARD_VM_CONTRACT;
const AssemblyNameIndex *assemblyNameIndex = m_assemblySimpleNameToIndexMap.LookupPtr(assembly->GetSimpleName());
if (assemblyNameIndex != nullptr)
{
VolatileStore(&m_pNativeMetadataAssemblyRefMap[assemblyNameIndex->Index], assembly);
}
}
#endif
#ifndef DACCESS_COMPILE
Assembly *NativeImage::LoadManifestAssembly(uint32_t rowid, DomainAssembly *pParentAssembly)
{
......
......@@ -94,7 +94,6 @@ private:
private:
NativeImage(AssemblyBinder *pAssemblyBinder, PEImageLayout *peImageLayout, LPCUTF8 imageFileName);
void AddComponentAssemblyToCache(Assembly *assembly);
protected:
void Initialize(READYTORUN_HEADER *header, LoaderAllocator *loaderAllocator, AllocMemTracker *pamTracker);
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册