Fix deadlock in ReadyToRun scenarios between EH and code heap deletion

[adityamandaleeka]

If the caller to ReadyToRunJitManager::JitCodeToMethodInfo doesn't care about the MethodDesc or EECodeInfo for the code (and passes in NULL for both), we shouldn't be doing any lookups to determine the method desc.

Turns out this pattern is used every time we call IsManagedCode in the ReadyToRun case (since we just want to know whether the code is managed or not) so this will avoid some extra work.

Also, this fixes the bug reported in #9745 where the pointless lookup causes the following deadlock (copied from my comment in that thread):

When the LoaderAllocator gets destroyed, it calls LoaderAllocator::GCLoaderAllocators which tries to delete unreferenced domain assemblies. The destructor for Assembly calls Assembly::Terminate which suspends the EE and then calls ExecutionManager::Unload. At this point, the ExecutionManager tries to delete code heaps, but in order to do so, it must acquire a writer lock (which in turn requires that there are no more readers active). This thread is stuck waiting here because...

...on another thread, System.Management.Automation.LocationGlobber.ExpandMshGlobPath threw an ItemNotFoundException, and we're in the process of dispatching that exception. One of the first things we need to do is unwind to the first managed call frame. This means checking if the code is managed and to do so, we first acquire the ExecutionManager's reader lock (because the scan flags for that thread tell us we need to). Now comes the part where ReadyToRun comes in: while we're holding the lock, we call JitCodeToMethodInfo, and the ReadyToRun version of that (ReadyToRunJitManager::JitCodeToMethodInfo) calls ReadyToRunInfo::GetMethodDescForEntryPoint which tries to do a hashmap lookup to find the MethodDesc corresponding to the entry point. However, HashMap::LookupValue tries to RareDisablePreemptiveGC before doing anything, and so the thread gets stuck while still holding the reader lock we got before.

Also, I couldn't help but fix some weird casing for locals in this function while I was touching it 😄

@jkotas PTAL

[jkotas]

It would be nice to do the cosmetic changes in separate change, so that this is easy to port to servicing branch.

Also, the same code is in NativeImageJitManager::JitCodeToMethodInfo as well.

[adityamandaleeka]

Good point, will do.

[jkotas]

It would be useful to add a comment here that this is also required to avoid deadlocks when this is called from IsManagedCode.

[adityamandaleeka]

@jkotas Updated.