Fix unloadability issue in AppDomain::FindAssembly

[janvorli]

There is a bug in AppDomain::FindAssembly code path that iterates over
the domain assemblies. The iteration loop leaves the refcount of the
LoaderAllocator related to the returned DomainAssembly bumped, but
nothing ever decrements it. So when a code that needs to be unloaded
ends up in that code path, all the managed things like managed
LoaderAllocator, LoaderAllocatorScout are destroyed, but the unloading
doesn't complete due to the refcount.

We have never found it before as this code path is never executed in any
of the coreclr tests even with unloadability testing option.

The problem was reported yesterday by @StephenMolloy who has found
it when working on making Xml serializer behave with unloadability.

[agocke]

@janvorli We're currently examining the risk for backporting #61266 (comment) and trying to decide if the value is worth it. It sounds like without this change, the XmlSerializer DLLs wouldn't be unloadable. If we were to bring this change back too, what would the risk look like?

[janvorli]

@agocke it is kind of hard to asses the risk here, even though I believe this was wrong before and the fix should not introduce any failure modes. But none of our coreclr or libraries tests execute this code path, so the only testing that was done was using the unload of the XmlSerializer where the unload was consistently failing before this fix and I believe it is consistently succeeding after the fix.
The problematic code path is specific to unloadability and the comment that was there indicated that there is no need for added reference, yet it was adding one via the not obvious handling of references in the CollectibleAssemblyHolder. So the original call to Extract instead of GetValue seems like an overlook of that fact.

[StephenMolloy]

@agocke and @janvorli, has there been any more conversation around porting this back to 6.0?

We do have customers who have been asking for quite some time for the XmlSerializer fix that tripped on this, and they have noticed it isn't in .Net 6 yet. While I do think there is value in the XmlSerializer fix on it's own, as it enables an experience that is less broken than the previous 'can't do this at all' story... having this fix in 6.0 would really be helpful, especially since 6.0 is LTS.

[ericstj]

Pinging @janvorli / @jkotas - what are your thoughts on shipping this in 6.0 servicing. Effectively adding to #61266

[jkotas]

I think it is to fine to ship this fix in 6.0 servicing, but I would like to see us to gain more confidence in the fix as part of it, e.g. create a small regression test that hits the bug and demonstrates that the bug is fixed with this change.