Revert "Use a ReaderWriterLockSlim in RcwCache"

[AaronRobinsonMSFT]

Reverts #117792

This introduced a functional regression reported in #120347.

Reopened #117775 performance regression issue introduced in #113907.

Fixes #119986
Fixes #120347

[Copilot]

Pull Request Overview

This PR reverts a previous change that introduced ReaderWriterLockSlim in the RcwCache class, replacing it back with the standard Lock implementation due to a functional regression. The change restores the simpler locking mechanism to address reported issues with the ReaderWriterLockSlim implementation.

Key changes:

• Reverts ReaderWriterLockSlim back to Lock with trivial waits optimization
• Simplifies locking code by removing explicit try-finally blocks and using lock statements
• Affects three methods: GetOrAddProxyForComInstance, FindProxyForComInstance, and Remove

[jkoritzinsky]

Can we use an upgradable read lock (that upgrades to a write lock around the Remove call)? Or does that not fix the problem?

If we can avoid the perf regression from getting rid of the rwlock, I'd prefer that over taking the perf regression (for .NET 11 at least).

[AaronRobinsonMSFT]

If we can avoid the perf regression from getting rid of the rwlock, I'd prefer that over taking the perf regression (for .NET 11 at least).

@jkoritzinsky Based on my comment at #117775 (comment), I don't think this fixed the regression either. At this point, I'm not sure we can address the regression in .NET 10 GA. We need to circle back and verify the actual regression though. The RPS system doesn't seem to be tracking the continued regression in this space.

[jkoritzinsky]

I could have sworn this fixed the regression when I ran the benchmark locally and in my PR. Since it didn't, yeah let's revert and revisit.

[AaronRobinsonMSFT]

/backport to release/10.0

[github-actions]

Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/18230714561

[rhuijben]

Here a readlock is not enough. It updates the hashtable under some circumstances, which may go concurrent under a read lock.

[rhuijben]

Upgrading to a write lock before the update (_cache.Remove()) may help too... But the single lock is probably just as easy.