Add ComWrappers RCW cache concurrency tests, and fix an RCW being handed out before it can be resolved

[Sergio0694]

The RCW cache had almost no concurrent coverage. Nine tests are added, covering the races that decide which RCW gets cached for a COM instance and what a caller is guaranteed once one comes back.

• Several threads miss the cache for one COM instance and all reach CreateObject. An implementation is free to hand each of them the same object, and only one of them can be published, so this checks that every thread comes back with the published one and that it still round trips to the COM instance it was created for.
• The same race with a distinct object per thread. The ones that lost have to be left exactly as they were rather than half registered: TryGetComInstance returns false rather than throwing, they can be the target of a weak reference, and they can still be registered as the wrapper for some other COM instance.
• Registering a caller supplied object races creating one over the same COM instance. Only one wins, every caller comes back with it, and the loser is untouched.
• A registration rejected over a dead cache entry has to leave the cache able to hand out a working RCW afterwards.
• An RCW has to be resolvable back to its COM instance the moment it is handed back. Every round gives all its threads a fresh COM instance to race over, which is the shape that reaches the window described below.
• The reference tracker registration. The other races use CreateObjectFlags.None, so the wrapper they build is not a tracker one and nothing reaches the tracker handle cache, which left that registration with no concurrent coverage. This one runs the same race with tracker objects and then checks what the registration is for, by handing the native object a thousand managed objects and collecting.
• Publishing, reading and removing cache entries from several threads at once while collections and finalizers run underneath.
• A dead entry replaced in place by the next RCW for the same COM instance, where for a while the entry belongs to a different wrapper than the one about to finalize.
• The cache clean up path for an RCW type with no finalizer, which gets a single GC handle that tracks resurrection rather than two, so its entries go dead at a different point in a collection than every other test in the file covers.

Eight of the nine pass on main. The ninth is the reason for the second commit.

The fix

An RCW is only resolvable back to its COM instance once its wrapper is in s_nativeObjectWrapperTable. That registration takes a lock covering every RCW in the process, so it cannot be done while holding a cache lock, which means the cache entry is published first and registered after. A lookup landing in that window returned an RCW that ComWrappers.TryGetComInstance then came up empty on.

Racing 16 threads to create the RCW for a fresh COM instance, over 8000 attempts, that happened 37 to 47 times per run on main, and never with this change.

The fix is to skip such an entry and report a miss. The creation path then finds that same entry under the cache write lock and registers it before returning it, which is a path that already runs today for every thread that loses the race, so no new behaviour is introduced. The cost is one predictable field read on the lookup path, plus one extra CreateObject call in a race that is rare by construction.

Performance

No measurable change. Manual harness over plain COM objects, p10 of 50 to 100 samples per scenario, alternating between two runtime layouts that differ only in System.Private.CoreLib.

Scenario	main	this PR
lookup, 1 instance	69.3 ns	68.9 ns	-0.6%
lookup, 64 instances	74.4 ns	75.1 ns	+0.9%
lookup, 1024 instances	81.0 ns	81.2 ns	+0.2%
create, 4000 fresh instances	563.5 ns	550.5 ns	-2.3%

The concurrent scenarios are not reported because they are not measurable on this machine: the run to run spread on the baseline alone is over 60%, which is far wider than any effect this change could have.

Validation

• All 31 tests in the ComWrappers API suite pass, over nine consecutive runs of a checked runtime.
• On main the same suite fails nine out of nine runs, always at the same test.
• All 30 COM interop suites were run against both. The 24 that pass on this machine pass identically with and without the change; the other 6 need a registered COM server and fail the same way on main.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

[Copilot]

🟡 Changes recommended

One of the newly added concurrency tests can hang indefinitely due to an unbounded Barrier.SignalAndWait() on both worker and main threads, so it should be made fail-fast with timeouts before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR strengthens COM ComWrappers RCW cache concurrency coverage by adding several new race-focused tests, and updates the CoreLib RCW cache lookup path to avoid handing out cache entries whose wrappers are not yet resolvable by ComWrappers.TryGetComInstance.

Changes:

• Add multiple new concurrent/GC-stress tests for RCW cache publish/read/remove races and registration races.
• Update RCW cache lookup to treat “published but not yet wrapper-table-registered” entries as a miss, and mark wrappers as registered after wrapper-table registration.
• Extend the internal RCW cache lookup helper to also return the owning NativeObjectWrapper so the lookup can validate registration state.

File summaries

File	Description
src/tests/Interop/COM/ComWrappers/API/Program.cs	Adds new concurrency and GC/finalizer stress tests for RCW caching and registration races.
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs	Skips handing out RCW cache entries until the wrapper is registered (resolvable), and plumbs wrapper info through the cache lookup.

Review details

Suppressed comments (1)

src/tests/Interop/COM/ComWrappers/API/Program.cs:727

• The main thread also waits on the start barrier without a timeout. If any worker doesn't reach the barrier, this will hang the test run rather than failing fast. Use a bounded wait and assert success.

 start.SignalAndWait();

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The added tests currently contain a brace imbalance (won’t compile) and one barrier wait on the main thread is unbounded, which can hang the test run.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/tests/Interop/COM/ComWrappers/API/Program.cs:727

• start.SignalAndWait() on the main thread has no timeout, so if a worker fails before reaching the barrier (e.g., due to an exception) the test can hang indefinitely. Use the same bounded wait pattern as the worker threads.

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

A new test stores null into a non-nullable object[], which can fail builds under nullable + TreatWarningsAsErrors and should be corrected before merge.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/tests/Interop/COM/ComWrappers/API/Program.cs:767

• supplied is declared as object[] but later stores null values. If nullable warnings are enabled (common in this repo) this can produce CS8625 under TreatWarningsAsErrors; and even without nullable, the type doesn’t reflect the intended contents. Use a nullable element type instead.

This issue also appears on line 823 of the same file.

src/tests/Interop/COM/ComWrappers/API/Program.cs:823

• After making supplied nullable, the foreach variable should also be nullable to avoid warnings from iterating a object?[].

 foreach (object candidate in supplied)

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[jkoritzinsky]

One comment on tests. Other than that LGTM

[Copilot]

🟡 Changes recommended

Some newly added barrier waits ignore the timeout result, which can silently weaken the concurrency tests and increase flakiness risk.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Several newly added concurrency tests ignore Barrier.SignalAndWait(TimeSpan) timeout results (risking silent desynchronization) and one assertion uses Assert.Equal where reference identity (Assert.Same) is required to validate the intended guarantee.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

src/tests/Interop/COM/ComWrappers/API/Program.cs:599

• Barrier.SignalAndWait(TimeSpan) returns false on timeout; ignoring the return risks this test silently continuing without actually synchronizing threads, reducing the chance of hitting the intended race (and potentially masking deadlocks/timeouts as non-failures).

This issue also appears in the following locations of the same file:

• line 786
• line 1056
• line 1158
  src/tests/Interop/COM/ComWrappers/API/Program.cs:1568

• The comment says the second lookup should return the same wrapper instance; Assert.Equal allows Equals-based success even if a different instance is returned. Assert.Same matches the intended guarantee.

src/tests/Interop/COM/ComWrappers/API/Program.cs:786

• Barrier.SignalAndWait(TimeSpan) can return false on timeout; ignoring the result can let the race test proceed without the intended synchronization, weakening the test and potentially hiding timeouts.

 barrier.SignalAndWait(TimeSpan.FromMinutes(1));

src/tests/Interop/COM/ComWrappers/API/Program.cs:1056

• Barrier.SignalAndWait(TimeSpan) returns a bool; ignoring it can allow CreateObject to continue after a timeout, weakening the guarantee that all threads reached this point and making the race assertions less reliable.

 _barrier.SignalAndWait(TimeSpan.FromMinutes(1));

src/tests/Interop/COM/ComWrappers/API/Program.cs:1158

• Barrier.SignalAndWait(TimeSpan) returns a bool; ignoring it can allow threads to proceed without actually synchronizing, which can make this race test pass without having exercised the intended interleaving.

 _barrier.SignalAndWait(TimeSpan.FromMinutes(1));

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Several newly added concurrency tests ignore Barrier.SignalAndWait(...) timeout results, which can undermine the intended synchronization and produce false positives or confusing failures.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

src/tests/Interop/COM/ComWrappers/API/Program.cs:599

• The barrier wait result is ignored. If the wait times out, the test proceeds without synchronizing threads, which can turn this into a false pass or produce confusing follow-on failures. Assert that the barrier phase completed so a timeout becomes a deterministic test failure.

This issue also appears on line 786 of the same file.

src/tests/Interop/COM/ComWrappers/API/Program.cs:786

• The barrier wait result is ignored. If the wait times out, the threads won't be aligned for the race being tested, which undermines the test's assertions. Assert the barrier completed (or otherwise fail fast) on timeout.

 barrier.SignalAndWait(TimeSpan.FromMinutes(1));

src/tests/Interop/COM/ComWrappers/API/Program.cs:1158

• The barrier wait return value is ignored. If the wait times out, the test no longer guarantees all threads missed the cache and are racing to publish, which is what the test intends to validate. Assert the barrier completed on each call.

 _barrier.SignalAndWait(TimeSpan.FromMinutes(1));

src/tests/Interop/COM/ComWrappers/API/Program.cs:1056

• This barrier wait is intended to be bounded so the test fails rather than hangs, but the return value is ignored. If it times out, the method still returns a proxy and the test may incorrectly continue. Assert the barrier completed.

 _barrier.SignalAndWait(TimeSpan.FromMinutes(1));

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite