Exit the lock before we call into user code and handle losing the race for the RCW table

[jkoritzinsky]

Fixes #110823

[jkoritzinsky]

Tests for this specific deadlock are still TODO, but I've reorganized the code for the NativeAOT ComWrappers implementation to match the same order of logic as the CoreCLR implementation and encapsulated each step into separate helper functions (which also helped significantly clarify lock usage).

[Sergio0694]

Thank you for adding all the comments!! 😄
Left a few more notes.

[jkoritzinsky]

Validated that the ComWrappers tests pass locally and added a test for the deadlock. This is ready for review!

[AaronRobinsonMSFT]

@jkoritzinsky Can you please run GCStress locally? Ideally we'd also include some amount of JITStress too. I'm also fine kicking off the legs that run too.

[jkoritzinsky]

Do we have GCStress support for NativeAOT?

[AaronRobinsonMSFT]

@jkoritzinsky Oh, right. I think we do, but we don't have a CI for it. @MichalStrehovsky Do we have a way to run GCStress on local native AOT?

[jkotas]

There is no GC stress for native AOT.

[jkoritzinsky]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Sergio0694]

Not exactly the same (although @EgorBo said it's kinda close enough? 😅), I've added an option in the Microsoft Store to simulate GC stress (I basically just spawn a thread that just spams GC.Collect(); Thread.Wait(50); in a loop). Perhaps it'd help if I also tested with that mode enabled for a bit, just to double check things don't blow up when that's running?

[jkoritzinsky]

NativeAOT outerloop failures are unrelated. Can I get one more review pass?

[Sergio0694]

I tested with the Microsoft Store and everything seems to work fine:

• Checked out release/9.0-staging (f580364)
• Cherry-pick Don't wait for finalizers in 'IReferenceTrackerHost::ReleaseDisconnectedReferenceSources' #110551
• Cherry-pick Fix calling convention mismatch in GC callouts #110685
• Cherry-pick Exit the lock before we call into user code and handle losing the race for the RCW table #110828
• Cherry-pick Move ComWrappers AddRef to C/C++ #110762
  • Branch with all changes: https://github.com/Sergio0694/runtime/commits/user/sergiopedri/store-testing/
• Build with .\build.cmd clr.aot -c Release
• Set IlcSdkPath in the Store .csproj to runtime\artifacts\bin\coreclr\windows.x64.Release\aotsdk\
• Set GenerateAppxPackageOnBuild
• Switch to Release
• Deploy

I used the Store for a while, including closing and launching it a few times, with and without our "GC stress" mode enabled as well (which creates a thread that just spams GC.Collect() every 50 milliseconds in a loop). Couldn't find any issues 🎉

Could we backport this one too? I think it should be our last blocker from the NAOT side for shipping 😄
Thank you!

[jkoritzinsky]

/backport to release/9.0-staging

[github-actions]

Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/12654501932