Free LOADERHANDLEDispatchMemberInfo dtor

[AaronRobinsonMSFT]

This is a regression from NETFX with the introduction of collectable assemblies
for COM scenarios in .NET Core 3.1 (dotnet/coreclr#20919).

Manually verified the user scenario and the references are no longer present.

Fixes#76350

[AaronRobinsonMSFT]

/cc @dotnet/interop-contrib

[janvorli]

LGTM, thank you for fixing it. I don't know how could I miss the handle freeing when making the change that introduced this problem.

[AaronRobinsonMSFT]

/backport to release/7.0

[github-actions]

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3341033443

[janvorli]

@AaronRobinsonMSFT I have just found that the freeing of the handle added in this PR causes assert in debug builds / crash in release ones during unloading. The problem is that at the time the DispatchMemberInfo destructor is called, the LoaderAllocator's handle table is already gone. It is stored in a managed array and a reference to it is stored in the managed LoaderAllocator.
We assert here in a callchain of the LoaderAllocator::FreeHandle:

runtime/src/coreclr/vm/loaderallocator.cpp

Lines 984 to 989 in b33aede

	_ASSERTE(!ObjectHandleIsNull(m_hLoaderAllocatorObjectHandle));
	UINT_PTR index = (((UINT_PTR)handle) >> 1) - 1;
	LOADERALLOCATORREF loaderAllocator = (LOADERALLOCATORREF)ObjectFromHandle(m_hLoaderAllocatorObjectHandle);
	PTRARRAYREF handleTable = loaderAllocator->GetHandleTable();
	handleTable->SetAt(index, value);

Here is the relevant part of the call stack:

 # Child-SP RetAddr Call Site
00 000000ee`a107b728 00007ffb`ff20902e KERNELBASE!DebugBreak+0x2
01 000000ee`a107b730 00007ffb`fe9ddfc3 coreclr!DbgAssertDialog+0x34e [F:\git\runtime7\src\coreclr\utilcode\debug.cpp @ 464] 02 000000ee`a107b970 00007ffb`fe9cfbc8 coreclr!LoaderAllocator::SetHandleValue+0x393 [F:\git\runtime7\src\coreclr\vm\loaderallocator.cpp @ 984] 03 000000ee`a107bc70 00007ffb`fefd8be8 coreclr!LoaderAllocator::FreeHandle+0x2b8 [F:\git\runtime7\src\coreclr\vm\loaderallocator.cpp @ 880] 04 000000ee`a107be40 00007ffb`fefd9488 coreclr!DispatchMemberInfo::~DispatchMemberInfo+0x328 [F:\git\runtime7\src\coreclr\vm\dispatchinfo.cpp @ 144] 05 000000ee`a107bfa0 00007ffb`fefd883e coreclr!DispatchMemberInfo::`scalar deleting destructor'+0x18
06 000000ee`a107bfd0 00007ffb`fefd9448 coreclr!DispatchInfo::~DispatchInfo+0x1ee [F:\git\runtime7\src\coreclr\vm\dispatchinfo.cpp @ 1065] 07 000000ee`a107c110 00007ffb`fef8db8a coreclr!DispatchInfo::`scalar deleting destructor'+0x18
08 000000ee`a107c140 00007ffb`fefaf523 coreclr!ComMethodTable::Cleanup+0x2da [F:\git\runtime7\src\coreclr\vm\comcallablewrapper.cpp @ 3105] 09 000000ee`a107c290 00007ffb`fef8d84a coreclr!ComMethodTable::Release+0x2e3 [F:\git\runtime7\src\coreclr\vm\comcallablewrapper.h @ 435] 0a 000000ee`a107c450 00007ffb`fefaf1ee coreclr!ComCallWrapperTemplate::Cleanup+0x4fa [F:\git\runtime7\src\coreclr\vm\comcallablewrapper.cpp @ 3907] 0b 000000ee`a107c730 00007ffb`fe8171da coreclr!ComCallWrapperTemplate::Release+0x2ae [F:\git\runtime7\src\coreclr\vm\comcallablewrapper.cpp @ 3938] 0c 000000ee`a107c8c0 00007ffb`fe7e5ec9 coreclr!EEClass::Destruct+0x3aa [F:\git\runtime7\src\coreclr\vm\class.cpp @ 136] 0d 000000ee`a107cf00 00007ffb`fe7df081 coreclr!Module::FreeClassTables+0x409 [F:\git\runtime7\src\coreclr\vm\ceeload.cpp @ 2016] 0e 000000ee`a107d200 00007ffb`fe8c9c81 coreclr!Module::Destruct+0x441 [F:\git\runtime7\src\coreclr\vm\ceeload.cpp @ 669] 0f 000000ee`a107d9d0 00007ffb`fe844581 coreclr!EditAndContinueModule::Destruct+0x41 [F:\git\runtime7\src\coreclr\vm\encee.cpp @ 83] 10 000000ee`a107da00 00007ffb`fe8382ef coreclr!ClassLoader::FreeModules+0x3c1 [F:\git\runtime7\src\coreclr\vm\clsload.cpp @ 1885] 11 000000ee`a107dc60 00007ffb`fe77b6f8 coreclr!ClassLoader::~ClassLoader+0x29f [F:\git\runtime7\src\coreclr\vm\clsload.cpp @ 1947] 12 000000ee`a107de10 00007ffb`fe78c6b3 coreclr!ClassLoader::`scalar deleting destructor'+0x18
13 000000ee`a107de40 00007ffb`fe77a341 coreclr!Assembly::Terminate+0x123 [F:\git\runtime7\src\coreclr\vm\assembly.cpp @ 310] 14 000000ee`a107df30 00007ffb`fe77b638 coreclr!Assembly::~Assembly+0x191 [F:\git\runtime7\src\coreclr\vm\assembly.cpp @ 243] 15 000000ee`a107e050 00007ffb`fe894666 coreclr!Assembly::`scalar deleting destructor'+0x18
16 000000ee`a107e080 00007ffb`fe726ed8 coreclr!DomainAssembly::~DomainAssembly+0x336 [F:\git\runtime7\src\coreclr\vm\domainassembly.cpp @ 91] 17 000000ee`a107e250 00007ffb`fe9cffff coreclr!DomainAssembly::`scalar deleting destructor'+0x18
18 000000ee`a107e280 00007ffb`fe9ce856 coreclr!LoaderAllocator::GCLoaderAllocators+0x39f [F:\git\runtime7\src\coreclr\vm\loaderallocator.cpp @ 565] 19 000000ee`a107e4e0 00007ffb`fe9df922 coreclr!LoaderAllocator::Destroy+0x3b6 [F:\git\runtime7\src\coreclr\vm\loaderallocator.cpp @ 698] 1a 000000ee`a107e6b0 00007ffb`5e288679 coreclr!LoaderAllocator_Destroy+0x212 [F:\git\runtime7\src\coreclr\vm\loaderallocator.cpp @ 714] 1b 000000ee`a107e8a0 00007ffb`5e2886c7 System_Private_CoreLib!System.Reflection.LoaderAllocatorScout.Destroy+0x39 [F:\git\runtime7\src\coreclr\System.Private.CoreLib\Microsoft.Interop.LibraryImportGenerator\Microsoft.Interop.LibraryImportGenerator\LibraryImports.g.cs @ 1115] 1c 000000ee`a107e970 00007ffb`ff191fe6 System_Private_CoreLib!System.Reflection.LoaderAllocatorScout.Finalize+0x17 [F:\git\runtime7\src\coreclr\System.Private.CoreLib\src\System\Reflection\LoaderAllocator.cs @ 39] 1d 000000ee`a107e9a0 00007ffb`fea21f45 coreclr!FastCallFinalizeWorker+0x6

The LoaderAllocator_Destroy is called from the finalizer of the LoaderAllocatorScout, which in turn is invoked as a result of the managed LoaderAllocator being collected.

[AaronRobinsonMSFT]

@AaronRobinsonMSFT I have just found that the freeing of the handle added in this PR causes assert in debug builds / crash in release ones during unloading.

Do we have a test for this? I'm not entirely sure how to fix this without a bit of time. Is this something that needs intimate knowledge of the assembly unloading logic or should I be able to muddle through with a fix?

[AaronRobinsonMSFT]

Created new issue - #79588