Fix ILStubCache allocation for collectible assemblies #21188
Conversation
Why do we need to keep the per AppDomain instance for compilation domain? Can everything use just the one on the LoaderAllocator? |
|
Can we add regression test for this? |
I've said "per module", not per AppDomain. I don't actually know the reason why the compilation domain requires per module cache. |
|
Just guessing ... maybe it was persisted by fragile NGen at some point in the past, but it is not today - there is "ZeroPointerField(this, offsetof(Module, m_pILStubCache))" in the fragile NGen path. i would clean it up and just have the one on the LoaderAllocator. |
|
Ok, sounds good. As for the regression test, I have a COM interop test with unloadability enabled that serves as a regression test (I've hit it in when I wanted to add that test to the collectible COM interop enabling PR and so I had to temporarily remove it). I will add it again after this PR is merged either as part of the COM interop PR or as a separate PR. |
|
Actually, the per-module cache may be needed for the appdomain cleanups that I am working on. I think this is good as is. |
|
@dotnet-bot test Ubuntu x64 Checked Innerloop Build and Test |
|
@dotnet-bot test Ubuntu x64 Checked Innerloop Build and Test (Jit - TieredCompilation=0) please |
|
@dotnet-bot test Ubuntu x64 Checked Innerloop Build and Test (Jit - TieredCompilation=0) please |
The ILStubCache was being allocated per domain unless the domain was a compilation AppDomain. This is wrong for collectible assemblies, since after an assembly is collected, the cache keeps stale entries referring to already deleted MethodTables. The fix is to make ILStubChange per LoaderAllocator instead (and keep the per module instances for compilation AppDomain).
janvorli
force-pushed
the
fix-collectible-ilstub-cache
branch
from
1461453
to
ef4b6f5
Compare
|
@dotnet-bot test Ubuntu arm Cross Checked Innerloop Build and Test please |
|
@dotnet-bot test this please |
|
@dotnet-bot test Ubuntu arm Cross Checked Innerloop Build and Test please |
…#21188) The ILStubCache was being allocated per domain unless the domain was a compilation AppDomain. This is wrong for collectible assemblies, since after an assembly is collected, the cache keeps stale entries referring to already deleted MethodTables. The fix is to make ILStubChange per LoaderAllocator instead (and keep the per module instances for compilation AppDomain). Commit migrated from dotnet/coreclr@8aa0869
The ILStubCache was being allocated per domain unless the domain was a
compilation AppDomain. This is wrong for collectible assemblies, since
after an assembly is collected, the cache keeps stale entries referring
to already deleted MethodTables.
The fix is to make ILStubChange per LoaderAllocator instead (and keep
the per module instances for compilation AppDomain).