diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs
index 56b882e88c8c65..435dac24bc5964 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs
@@ -634,7 +634,14 @@ protected NativeObjectWrapper(IntPtr externalComObject, IntPtr inner, ComWrapper
internal IntPtr ExternalComObject => _externalComObject;
internal ComWrappers ComWrappers => _comWrappers;
+
+ ///
+ /// The handle to the RCW this wrapper tracks. This wrapper owns it, and is what frees it, but the RCW
+ /// cache stores copies of it as its entries rather than allocating a second handle per RCW, so it must
+ /// outlive every entry referring to it.
+ ///
internal WeakGCHandle ProxyHandle => _proxyHandle;
+
internal bool IsUniqueInstance => _uniqueInstance;
internal bool IsAggregatedWithManagedObjectWrapper => _aggregatedManagedObjectWrapper;
@@ -642,6 +649,8 @@ public virtual void Release()
{
if (!_uniqueInstance && _comWrappers is not null)
{
+ // The RCW cache shares this handle rather than allocating one of its own, so the entry has to be
+ // dropped before the handle is freed below, or the cache would be left holding a freed handle.
_comWrappers._rcwCache.Remove(_externalComObject, this);
_comWrappers = null!;
}
@@ -1253,12 +1262,17 @@ private object RegisterObjectForComInstance(
flags,
ref referenceTrackerMaybe);
- object actualProxy = comProxy;
NativeObjectWrapper actualWrapper = nativeObjectWrapper;
+ object actualProxy = comProxy;
+ bool reserved = false;
+
if (!nativeObjectWrapper.IsUniqueInstance)
{
- // Add our entry to the cache here, using an already existing entry if someone else beat us to it.
- (actualWrapper, actualProxy) = _rcwCache.GetOrAddProxyForComInstance(identity, nativeObjectWrapper, comProxy);
+ // Reserve the entry for this COM instance, or pick up the one another thread got in first
+ // with. The entry stays marked as pending until the registration below has run, because
+ // that is what lets an entry be resolved back to the wrapper tracking its RCW.
+ (actualWrapper, actualProxy, reserved) = _rcwCache.GetOrAddProxyForComInstance(identity, nativeObjectWrapper, comProxy);
+
if (actualWrapper != nativeObjectWrapper)
{
// We raced with another thread to map identity to nativeObjectWrapper
@@ -1267,45 +1281,50 @@ private object RegisterObjectForComInstance(
}
}
- // At this point, actualProxy is the RCW object for the identity
- // and actualWrapper is the NativeObjectWrapper that is in the RCW cache (if not unique) that associates the identity with actualProxy.
- // Register the NativeObjectWrapper to handle lifetime tracking of the references to the COM object.
- RegisterWrapperForObject(actualWrapper, actualProxy);
+ // Register the RCW so it can be resolved back to the wrapper tracking it. Every thread that gets
+ // here for the same COM instance does this with the same pair, so whichever arrives first wins and
+ // the rest are no-ops. This is deliberately not done while holding a cache lock: it takes a lock
+ // covering the whole table, and holding a bucket lock across that would put every bucket behind it.
+ try
+ {
+ NativeObjectWrapper registeredWrapper = s_nativeObjectWrapperTable.GetOrAdd(actualProxy, actualWrapper);
- return actualProxy;
- }
+ if (registeredWrapper != actualWrapper)
+ {
+ // The object 'CreateObject' handed back is already the RCW for another COM instance, which is
+ // not something ComWrappers can represent. Releasing the wrapper also drops the entry reserved
+ // for it above, if this thread is the one that reserved it.
+ Debug.Assert(registeredWrapper.ExternalComObject != actualWrapper.ExternalComObject);
- private void RegisterWrapperForObject(NativeObjectWrapper wrapper, object comProxy)
- {
- // When we call into RegisterWrapperForObject, there is only one valid non-"unique instance" wrapper for a given
- // COM instance, which is already registered in the RCW cache.
- // If we find a wrapper in the table that is a different NativeObjectWrapper instance
- // then it must be for a different COM instance.
- // It's possible that we could race here with another thread that is trying to register the same comProxy
- // for the same COM instance, but in that case we'll be passed the same NativeObjectWrapper instance
- // for both threads. In that case, it doesn't matter which thread adds the entry to the NativeObjectWrapper table
- // as the entry is always the same pair.
- Debug.Assert(wrapper.ProxyHandle.TryGetTarget(out object? proxyTarget) && proxyTarget == comProxy);
- Debug.Assert(wrapper.IsUniqueInstance || _rcwCache.FindProxyForComInstance(wrapper.ExternalComObject) == comProxy);
-
- // Add the input wrapper bound to the COM proxy, if there isn't one already. If another thread raced
- // against this one and this lost, we'd get the wrapper added from that thread instead.
- NativeObjectWrapper registeredWrapper = s_nativeObjectWrapperTable.GetOrAdd(comProxy, wrapper);
-
- // We lost the race, so we cannot register the incoming wrapper with the target object
- if (registeredWrapper != wrapper)
- {
- Debug.Assert(registeredWrapper.ExternalComObject != wrapper.ExternalComObject);
- wrapper.Release();
- throw new NotSupportedException();
- }
-
- // Always register our wrapper to the reference tracker handle cache here.
- // We may not be the thread that registered the handle, but we need to ensure that the wrapper
- // is registered before we return to user code. Otherwise the wrapper won't be walked by the
- // TrackerObjectManager and we could end up missing a section of the object graph.
- // This cache deduplicates, so it is okay that the wrapper will be registered multiple times.
- AddWrapperToReferenceTrackerHandleCache(registeredWrapper);
+ actualWrapper.Release();
+
+ throw new NotSupportedException();
+ }
+
+ // At this point, actualProxy is the RCW object for the identity
+ // and actualWrapper is the NativeObjectWrapper that is in the RCW cache (if not unique) that associates the identity with actualProxy.
+ //
+ // Always register our wrapper to the reference tracker handle cache here.
+ // We may not be the thread that registered the handle, but we need to ensure that the wrapper
+ // is registered before we return to user code. Otherwise the wrapper won't be walked by the
+ // TrackerObjectManager and we could end up missing a section of the object graph.
+ // This cache deduplicates, so it is okay that the wrapper will be registered multiple times.
+ AddWrapperToReferenceTrackerHandleCache(actualWrapper);
+ }
+ finally
+ {
+ if (reserved)
+ {
+ // The reservation is dropped whether the registration went through or not, because leaving
+ // one behind would keep its wrapper alive, and a wrapper that outlives its RCW can never be
+ // finalized, which is what removes the entry. Dropping it is right either way: if some other
+ // thread picked the wrapper up and registered it, the entry is now usable, and if nobody did,
+ // nothing is left referring to the wrapper, so it is finalized and takes the entry with it.
+ _rcwCache.CommitProxyForComInstance(identity, nativeObjectWrapper);
+ }
+ }
+
+ return actualProxy;
}
private static void AddWrapperToReferenceTrackerHandleCache(NativeObjectWrapper wrapper)
@@ -1322,13 +1341,23 @@ internal void RemoveWrappersFromCache(IEnumerable wrappers)
}
///
- /// The cache mapping COM instances to the objects tracking their RCWs.
+ /// The cache mapping COM instances to the RCWs wrapping them.
///
///
+ ///
+ /// An entry names the RCW rather than the tracking it, and does so through
+ /// a copy of the handle that wrapper already keeps to it, so that a cached RCW costs no handle of its own.
+ /// The wrapper for a cached RCW is resolved through , so an entry is
+ /// reserved before its RCW is registered there and only becomes usable once it has been. Registering is
+ /// deliberately left outside the bucket lock: it takes a lock covering that whole table, and holding a
+ /// bucket lock across it would serialize every bucket behind a single process wide lock.
+ ///
+ ///
/// The cache is partitioned into several independent buckets, each with its own lock, so that operations on
/// COM instances that map to different buckets don't contend with one another. Reducing that contention is
/// important because the cache is consulted on essentially every transition from native to managed code, and
/// because the finalizer thread concurrently takes write locks to remove entries for collected RCWs.
+ ///
///
private readonly struct RcwCache
{
@@ -1381,17 +1410,33 @@ private unsafe ref readonly Bucket GetBucket(IntPtr comPointer)
}
///
- /// Gets the current RCW proxy object for if it exists in the cache or inserts a new entry with .
+ /// Reserves the entry for for , or gets the RCW another thread has already put there.
///
/// The com instance we want to get or record an RCW for.
/// The for .
/// The proxy object that is associated with .
- /// The proxy object currently in the cache for or the proxy object owned by if no entry exists and the corresponding native wrapper.
- public (NativeObjectWrapper actualWrapper, object actualProxy) GetOrAddProxyForComInstance(IntPtr comPointer, NativeObjectWrapper wrapper, object comProxy)
+ ///
+ /// The proxy object currently in the cache for and the wrapper tracking it, or
+ /// and if this call is the one that reserved the entry.
+ /// The last item says which of the two happened, and is when the entry was reserved here,
+ /// in which case the caller must complete it with or release
+ /// .
+ ///
+ public (NativeObjectWrapper actualWrapper, object actualProxy, bool reserved) GetOrAddProxyForComInstance(IntPtr comPointer, NativeObjectWrapper wrapper, object comProxy)
{
return GetBucket(comPointer).GetOrAddProxyForComInstance(comPointer, wrapper, comProxy);
}
+ ///
+ /// Marks the entry reserved for as usable, once its RCW has been registered.
+ ///
+ /// The com instance to complete the entry for.
+ /// The that reserved the entry.
+ public void CommitProxyForComInstance(IntPtr comPointer, NativeObjectWrapper wrapper)
+ {
+ GetBucket(comPointer).CommitProxyForComInstance(comPointer, wrapper);
+ }
+
///
/// Gets the current RCW proxy object for , if it exists in the cache and is still alive.
///
@@ -1441,7 +1486,7 @@ public void RemoveAll(IEnumerable wrappers)
private readonly struct Bucket
{
private readonly ReaderWriterLockSlim _lock;
- private readonly Dictionary> _cache;
+ private readonly Dictionary _cache;
public Bucket()
{
@@ -1449,43 +1494,69 @@ public Bucket()
_cache = [];
}
+ ///
+ /// An entry in a bucket: the RCW cached for a COM instance, plus the wrapper publishing it for as
+ /// long as that publication is still in flight.
+ ///
+ private struct Entry
+ {
+ ///
+ /// A copy of the handle its wrapper keeps to the RCW. That wrapper owns it and is what frees it.
+ ///
+ public WeakGCHandle ProxyHandle;
+
+ ///
+ /// The wrapper that reserved this entry, until it has registered its RCW in the wrapper table,
+ /// and from then on. It is carried here for the threads that come across
+ /// the entry during that window, as they cannot resolve it through the table yet. It has to be
+ /// dropped afterwards, because the cache may not be what keeps a wrapper alive: one that
+ /// outlived its RCW would never be finalized, and its finalizer is what removes this entry.
+ ///
+ public NativeObjectWrapper? PendingWrapper;
+ }
+
///
- public (NativeObjectWrapper actualWrapper, object actualProxy) GetOrAddProxyForComInstance(IntPtr comPointer, NativeObjectWrapper wrapper, object comProxy)
+ public (NativeObjectWrapper actualWrapper, object actualProxy, bool reserved) GetOrAddProxyForComInstance(IntPtr comPointer, NativeObjectWrapper wrapper, object comProxy)
{
_lock.EnterWriteLock();
try
{
Debug.Assert(wrapper.ProxyHandle.TryGetTarget(out object? proxyTarget) && proxyTarget == comProxy);
- ref WeakGCHandle rcwEntry = ref CollectionsMarshal.GetValueRefOrAddDefault(_cache, comPointer, out bool exists);
- if (!exists)
- {
- // Someone else didn't beat us to adding the entry to the cache.
- // Add our entry here.
- rcwEntry = new WeakGCHandle(wrapper);
- }
- else if (!rcwEntry.TryGetTarget(out NativeObjectWrapper? cachedWrapper))
- {
- Debug.Assert(rcwEntry.IsAllocated);
- // The target was collected, so we need to update the cache entry.
- rcwEntry.SetTarget(wrapper);
- }
- else
+
+ ref Entry rcwEntry = ref CollectionsMarshal.GetValueRefOrAddDefault(_cache, comPointer, out bool exists);
+
+ if (exists && rcwEntry.ProxyHandle.TryGetTarget(out object? existingProxy))
{
- // The target NativeObjectWrapper was not collected, but we need to make sure
- // that the proxy object is still alive.
- if (cachedWrapper.ProxyHandle.TryGetTarget(out object? existingProxy))
+ // Someone else beat us to the entry and their RCW is still alive, so that is the one to
+ // use. While they are still registering it, their wrapper is on the entry, because it
+ // cannot be reached through the table yet. Reading that table here is fine even though
+ // this lock is held: only mutating it takes its own lock, lookups are free of it.
+ NativeObjectWrapper? existingWrapper = rcwEntry.PendingWrapper;
+
+ if (existingWrapper is null)
+ {
+ _ = s_nativeObjectWrapperTable.TryGetValue(existingProxy, out existingWrapper);
+ }
+
+ if (existingWrapper is not null)
{
- // The existing proxy object is still alive, we will use that.
- return (cachedWrapper, existingProxy);
+ return (existingWrapper, existingProxy, false);
}
- // The proxy object was collected, so we need to update the cache entry.
- rcwEntry.SetTarget(wrapper);
+ // The entry names an RCW that never made it into the table, which is what a thread
+ // that failed part way through publishing leaves behind. It can't be resolved back to
+ // a wrapper, so it is of no use to anyone and ours replaces it below.
}
- // We either added an entry to the cache or updated an existing entry that was dead.
- // Return our target object.
- return (wrapper, comProxy);
+ // There was either no entry, or one whose RCW has been collected, so ours takes its place.
+ // The handle that gets overwritten is not freed here: every handle in this cache belongs to
+ // the wrapper that created it, which is what frees it, from 'NativeObjectWrapper.Release'.
+ // Reserving the slot now rather than once the registration has run means the only part of
+ // publishing that can fail on its own, growing the dictionary, has already happened by then.
+ rcwEntry.ProxyHandle = wrapper.ProxyHandle;
+ rcwEntry.PendingWrapper = wrapper;
+
+ return (wrapper, comProxy, true);
}
finally
{
@@ -1493,19 +1564,62 @@ public Bucket()
}
}
+ ///
+ public void CommitProxyForComInstance(IntPtr comPointer, NativeObjectWrapper wrapper)
+ {
+ // Completing a reservation only overwrites one reference field of one entry. It never adds or
+ // removes anything, so the dictionary's layout doesn't change and this doesn't have to exclude
+ // the lookups running alongside it, only the writers that could move the entry out from under
+ // it. The lookup that can overlap this is 'FindProxyForComInstance', which either still sees
+ // the reservation and reports a miss, sending its caller down a path that takes the write lock
+ // and resolves the entry properly, or sees it gone and hands out an RCW that is by then
+ // registered. Taking the read lock rather than the write lock is what keeps concurrent
+ // creations from serializing here after they have just been let through the write lock above.
+ _lock.EnterReadLock();
+ try
+ {
+ ref Entry rcwEntry = ref CollectionsMarshal.GetValueRefOrNullRef(_cache, comPointer);
+
+ // Only ever clear the reservation this wrapper made. Its RCW may have been collected while
+ // the registration ran, letting another wrapper take the entry over, and that one is still
+ // pending on its own registration.
+ if (!Unsafe.IsNullRef(ref rcwEntry) && ReferenceEquals(rcwEntry.PendingWrapper, wrapper))
+ {
+ rcwEntry.PendingWrapper = null;
+ }
+ }
+ finally
+ {
+ _lock.ExitReadLock();
+ }
+ }
+
///
public object? FindProxyForComInstance(IntPtr comPointer)
{
_lock.EnterReadLock();
try
{
- if (!_cache.TryGetValue(comPointer, out WeakGCHandle existingHandle))
+ // Read the entry in place rather than copying it out. It carries a reference as well as
+ // the handle now, and this runs on essentially every transition from native code, so the
+ // copy is worth avoiding. Holding the read lock is what makes the reference safe to use:
+ // it keeps out the writers that could move the entry.
+ ref Entry existingEntry = ref CollectionsMarshal.GetValueRefOrNullRef(_cache, comPointer);
+
+ if (Unsafe.IsNullRef(ref existingEntry))
{
// No entry in the cache.
return null;
}
- if (existingHandle.TryGetTarget(out NativeObjectWrapper? cachedWrapper)
- && cachedWrapper.ProxyHandle.TryGetTarget(out object? cachedProxy))
+ if (existingEntry.PendingWrapper is not null)
+ {
+ // The entry is reserved but its RCW is not registered yet, so it cannot be handed out:
+ // the thread that reserved it may still fail and release the wrapper backing it.
+ // Reporting it as absent sends the caller down the creation path, which resolves the
+ // entry under the write lock, where the wrapper that reserved it is reachable.
+ return null;
+ }
+ if (existingEntry.ProxyHandle.TryGetTarget(out object? cachedProxy))
{
// The target exists and is still alive. Return it.
return cachedProxy;
@@ -1525,13 +1639,15 @@ public Bucket()
{
// Someone else could have removed the entry or added a new one in the time
// between us releasing the read lock and acquiring the write lock.
- if (_cache.TryGetValue(comPointer, out WeakGCHandle existingHandle)
- && !existingHandle.TryGetTarget(out _))
+ ref Entry existingEntry = ref CollectionsMarshal.GetValueRefOrNullRef(_cache, comPointer);
+
+ if (!Unsafe.IsNullRef(ref existingEntry)
+ && existingEntry.PendingWrapper is null
+ && !existingEntry.ProxyHandle.TryGetTarget(out _))
{
- // There's still a dead entry in the cache,
- // remove it.
+ // There's still a dead entry in the cache, remove it. Only the entry is dropped, as
+ // the handle belongs to the wrapper that created it and is freed along with it.
_cache.Remove(comPointer);
- existingHandle.Dispose();
}
}
finally
@@ -1549,16 +1665,17 @@ public void Remove(IntPtr comPointer, NativeObjectWrapper wrapper)
try
{
// TryGetOrCreateObjectForComInstanceInternal may have put a new entry into the cache
- // in the time between the GC cleared the contents of the GC handle but before the
- // NativeObjectWrapper finalizer ran.
- // Only remove the entry if the target of the GC handle is the NativeObjectWrapper
- // or is null (indicating that the corresponding NativeObjectWrapper has been scheduled for finalization).
- if (_cache.TryGetValue(comPointer, out WeakGCHandle cachedRef)
- && (!cachedRef.TryGetTarget(out NativeObjectWrapper? cachedWrapper)
- || cachedWrapper == wrapper))
+ // in the time between the GC clearing the contents of the GC handle and the
+ // NativeObjectWrapper finalizer running. Only the entry this wrapper published may be
+ // removed, and comparing the handles rather than their targets identifies it exactly,
+ // including once the RCW it refers to has been collected. That covers an entry this
+ // wrapper only reserved as well, as a reservation already carries its handle.
+ ref Entry cachedEntry = ref CollectionsMarshal.GetValueRefOrNullRef(_cache, comPointer);
+
+ if (!Unsafe.IsNullRef(ref cachedEntry)
+ && cachedEntry.ProxyHandle.Equals(wrapper.ProxyHandle))
{
_cache.Remove(comPointer);
- cachedRef.Dispose();
}
}
finally
diff --git a/src/tests/Interop/COM/ComWrappers/API/Program.cs b/src/tests/Interop/COM/ComWrappers/API/Program.cs
index 233a476c4d747f..89c79a95072810 100644
--- a/src/tests/Interop/COM/ComWrappers/API/Program.cs
+++ b/src/tests/Interop/COM/ComWrappers/API/Program.cs
@@ -5,6 +5,7 @@ namespace ComWrappersTests
{
using System;
using System.Collections;
+ using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;
@@ -538,6 +539,738 @@ public void ValidateCreateObjectCachingScenario()
Assert.NotEqual(trackerObj1, trackerObj3);
}
+ private sealed unsafe class PlainProxyComWrappers : ComWrappers
+ {
+ protected override ComInterfaceEntry* ComputeVtables(object obj, CreateComInterfaceFlags flags, out int count)
+ {
+ count = 0;
+ return null;
+ }
+
+ protected override object CreateObject(IntPtr externalComObject, CreateObjectFlags flags) => new();
+
+ protected override void ReleaseObjects(IEnumerable objects) => throw new NotImplementedException();
+ }
+
+ // An RCW is only resolvable back to its COM instance once its wrapper is in the wrapper table, and
+ // that registration cannot be done while holding a cache lock. So an entry is reserved rather than
+ // published outright, and a reserved entry is not handed out, or a thread could be given an RCW
+ // that does not resolve yet. Every round here is a fresh COM instance that all the threads race to
+ // create the RCW for, which is the shape that hits it: without the reservation this fails in the
+ // low tens out of these several thousand attempts, and on a build with it, never.
+ [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
+ [Fact]
+ public void ValidateCreateObjectRaceResolvesImmediately()
+ {
+ Console.WriteLine($"Running {nameof(ValidateCreateObjectRaceResolvesImmediately)}...");
+
+ const int ThreadCount = 16;
+ const int RoundCount = 500;
+
+ IntPtr[] instances = new IntPtr[RoundCount];
+
+ for (int i = 0; i < instances.Length; i++)
+ {
+ instances[i] = MockReferenceTrackerRuntime.CreateTrackerObject();
+ }
+
+ var cw = new PlainProxyComWrappers();
+ var failures = new ConcurrentQueue();
+
+ // The RCWs are kept alive for the whole run, so that no wrapper is finalized underneath a
+ // round that is still being checked.
+ object[] proxies = new object[RoundCount];
+
+ using var barrier = new Barrier(ThreadCount);
+
+ int unresolved = 0;
+ var threads = new Thread[ThreadCount];
+
+ for (int t = 0; t < threads.Length; t++)
+ {
+ threads[t] = new Thread(() =>
+ {
+ for (int round = 0; round < RoundCount; round++)
+ {
+ try
+ {
+ // Line every thread up on the same instance, so they all miss the cache and
+ // race to publish it rather than finding each other's entries.
+ barrier.SignalAndWait(TimeSpan.FromMinutes(1));
+
+ object proxy = cw.GetOrCreateObjectForComInstance(instances[round], CreateObjectFlags.None);
+
+ proxies[round] = proxy;
+
+ // The RCW is in hand, so it has to name the COM instance it was created for.
+ if (ComWrappers.TryGetComInstance(proxy, out IntPtr unknown))
+ {
+ Marshal.Release(unknown);
+ }
+ else
+ {
+ Interlocked.Increment(ref unresolved);
+ }
+ }
+ catch (Exception e)
+ {
+ // Carry on to the next round regardless, so that the other threads are not
+ // left waiting on a barrier this one has stopped arriving at.
+ failures.Enqueue(e);
+ }
+ }
+ })
+ { IsBackground = true, Name = $"ComWrappers resolve {t}" };
+
+ threads[t].Start();
+ }
+
+ foreach (Thread thread in threads)
+ {
+ Assert.True(thread.Join(TimeSpan.FromMinutes(2)), "A worker thread did not finish, which suggests a deadlock while racing to publish.");
+ }
+
+ Assert.Empty(failures);
+ Assert.Equal(0, unresolved);
+
+ GC.KeepAlive(proxies);
+
+ foreach (IntPtr instance in instances)
+ {
+ Marshal.Release(instance);
+ }
+ }
+
+ // The RCW cache shares the GC handle that the NativeObjectWrapper keeps to its RCW, rather than
+ // allocating one of its own. That handle is freed by the wrapper, so this hammers the paths that
+ // publish, read and remove those entries from several threads at once, while collections and
+ // finalizers run underneath, to catch a cache entry ever being read after its owner freed it.
+ [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
+ [Fact]
+ public void ValidateCreateObjectConcurrentCacheAccess()
+ {
+ Console.WriteLine($"Running {nameof(ValidateCreateObjectConcurrentCacheAccess)}...");
+
+ const int ThreadCount = 8;
+ const int IterationCount = 300;
+ const int InstanceCount = 4;
+
+ var cw = new TestComWrappers();
+
+ IntPtr[] instances = new IntPtr[InstanceCount];
+ IntPtr[] identities = new IntPtr[InstanceCount];
+
+ for (int i = 0; i < instances.Length; i++)
+ {
+ instances[i] = MockReferenceTrackerRuntime.CreateTrackerObject();
+
+ // ComWrappers keys the cache on the identity IUnknown, which is what a round trip back to
+ // native produces, and that is not necessarily the pointer the object was created as.
+ Assert.Equal(0, Marshal.QueryInterface(instances[i], IUnknownVtbl.IID_IUnknown, out identities[i]));
+ }
+
+ using var start = new Barrier(ThreadCount + 1);
+ var failures = new ConcurrentQueue();
+ var threads = new Thread[ThreadCount];
+
+ for (int t = 0; t < threads.Length; t++)
+ {
+ int index = t;
+
+ threads[t] = new Thread(() =>
+ {
+ start.SignalAndWait();
+
+ try
+ {
+ for (int i = 0; i < IterationCount; i++)
+ {
+ // Every thread walks the instances from a different offset, so the threads are
+ // spread over the buckets rather than all queueing on one of them.
+ int slot = (i + index) % instances.Length;
+
+ var wrapper = (ITrackerObjectWrapper)cw.GetOrCreateObjectForComInstance(instances[slot], CreateObjectFlags.None);
+
+ Assert.NotNull(wrapper);
+
+ // Asking again while this thread still holds the wrapper has to produce the same
+ // object, which is the guarantee the cache exists to provide.
+ var again = (ITrackerObjectWrapper)cw.GetOrCreateObjectForComInstance(instances[slot], CreateObjectFlags.None);
+
+ Assert.Same(wrapper, again);
+
+ // Round trip it back to a native pointer, which reads the wrapper the cache entry
+ // resolves to, and has to name the instance this thread asked for.
+ Assert.True(ComWrappers.TryGetComInstance(wrapper, out IntPtr unknown));
+ Assert.Equal(identities[slot], unknown);
+ Marshal.Release(unknown);
+
+ if ((i % 16) == index % 16)
+ {
+ // Drop everything this thread is holding and collect, so that entries go dead
+ // and wrapper finalizers run while the other threads are still using the cache.
+ GC.Collect();
+ GC.WaitForPendingFinalizers();
+ }
+ }
+ }
+ catch (Exception e)
+ {
+ failures.Enqueue(e);
+ }
+ })
+ { IsBackground = true, Name = $"ComWrappers cache {index}" };
+
+ threads[t].Start();
+ }
+
+ start.SignalAndWait();
+
+ foreach (Thread thread in threads)
+ {
+ Assert.True(thread.Join(TimeSpan.FromMinutes(2)), "A worker thread did not finish, which suggests a deadlock in the RCW cache.");
+ }
+
+ Assert.Empty(failures);
+
+ ForceGC();
+
+ foreach (IntPtr identity in identities)
+ {
+ Marshal.Release(identity);
+ }
+
+ foreach (IntPtr instance in instances)
+ {
+ Marshal.Release(instance);
+ }
+ }
+
+ // Registering a caller supplied object and creating one race through the same cache, and only one
+ // of them can win. Whichever does, every caller has to come back with it, and the objects that
+ // lost have to be left exactly as they were rather than half registered.
+ [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
+ [Fact]
+ public void ValidateRegisterAndCreateRaceForSameComInstance()
+ {
+ Console.WriteLine($"Running {nameof(ValidateRegisterAndCreateRaceForSameComInstance)}...");
+
+ const int ThreadCount = 8;
+
+ IntPtr instanceRaw = MockReferenceTrackerRuntime.CreateTrackerObject();
+
+ Assert.Equal(0, Marshal.QueryInterface(instanceRaw, IUnknownVtbl.IID_IUnknown, out IntPtr identity));
+
+ var cw = new PlainProxyComWrappers();
+ var failures = new ConcurrentQueue();
+
+ object[] supplied = new object[ThreadCount];
+ object[] results = new object[ThreadCount];
+ var threads = new Thread[ThreadCount];
+
+ using var barrier = new Barrier(ThreadCount);
+
+ for (int t = 0; t < threads.Length; t++)
+ {
+ int index = t;
+
+ // Every other thread brings its own object to register, the rest ask for one to be created.
+ supplied[index] = (index % 2) == 0 ? new object() : null;
+
+ threads[t] = new Thread(() =>
+ {
+ try
+ {
+ barrier.SignalAndWait(TimeSpan.FromMinutes(1));
+
+ results[index] = supplied[index] is object toRegister
+ ? cw.GetOrRegisterObjectForComInstance(instanceRaw, CreateObjectFlags.None, toRegister)
+ : cw.GetOrCreateObjectForComInstance(instanceRaw, CreateObjectFlags.None);
+ }
+ catch (Exception e)
+ {
+ failures.Enqueue(e);
+ }
+ })
+ { IsBackground = true, Name = $"ComWrappers register race {index}" };
+
+ threads[t].Start();
+ }
+
+ foreach (Thread thread in threads)
+ {
+ Assert.True(thread.Join(TimeSpan.FromMinutes(2)), "A worker thread did not finish, which suggests a deadlock while racing to publish.");
+ }
+
+ Assert.Empty(failures);
+
+ object winner = results[0];
+
+ foreach (object result in results)
+ {
+ Assert.Same(winner, result);
+ }
+
+ Assert.True(ComWrappers.TryGetComInstance(winner, out IntPtr winnerUnknown));
+ Assert.Equal(identity, winnerUnknown);
+ Marshal.Release(winnerUnknown);
+
+ // A supplied object that lost has to look like an object that was never handed over at all,
+ // and has to still be usable as the wrapper for some other COM instance.
+ IntPtr otherRaw = MockReferenceTrackerRuntime.CreateTrackerObject();
+ bool reusedOne = false;
+
+ foreach (object candidate in supplied)
+ {
+ if (candidate is null || ReferenceEquals(candidate, winner))
+ {
+ continue;
+ }
+
+ Assert.False(ComWrappers.TryGetComInstance(candidate, out IntPtr loserUnknown));
+ Assert.Equal(IntPtr.Zero, loserUnknown);
+
+ if (!reusedOne)
+ {
+ Assert.Same(candidate, cw.GetOrRegisterObjectForComInstance(otherRaw, CreateObjectFlags.None, candidate));
+ reusedOne = true;
+ }
+ }
+
+ Marshal.Release(otherRaw);
+ Marshal.Release(identity);
+ Marshal.Release(instanceRaw);
+ }
+
+ // A registration is rejected when the object handed over is already the RCW for another COM
+ // instance. The existing coverage rejects one for a COM instance the cache has never seen; this
+ // one rejects it for a COM instance that still has an entry whose RCW has been collected, which
+ // is a different path through the cache because the entry is there and has to be taken over.
+ [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
+ [Fact]
+ public void ValidateRejectedRegistrationOverDeadEntry()
+ {
+ Console.WriteLine($"Running {nameof(ValidateRejectedRegistrationOverDeadEntry)}...");
+
+ var cw = new PlainProxyComWrappers();
+
+ IntPtr firstRaw = MockReferenceTrackerRuntime.CreateTrackerObject();
+ IntPtr secondRaw = MockReferenceTrackerRuntime.CreateTrackerObject();
+
+ // An RCW that belongs to the first COM instance, so registering it for another is rejected.
+ object owned = cw.GetOrCreateObjectForComInstance(firstRaw, CreateObjectFlags.None);
+
+ // Leave a dead entry behind for the second instance: collect its RCW without draining
+ // finalizers, so the entry is still there but no longer names anything.
+ CreateAndAbandon(cw, secondRaw);
+
+ GC.Collect();
+
+ Assert.Throws(() => cw.GetOrRegisterObjectForComInstance(secondRaw, CreateObjectFlags.None, owned));
+
+ // The rejection must not have disturbed the instance the object does belong to.
+ Assert.True(ComWrappers.TryGetComInstance(owned, out IntPtr ownedUnknown));
+
+ Assert.Equal(0, Marshal.QueryInterface(firstRaw, IUnknownVtbl.IID_IUnknown, out IntPtr firstIdentity));
+ Assert.Equal(firstIdentity, ownedUnknown);
+
+ Marshal.Release(firstIdentity);
+ Marshal.Release(ownedUnknown);
+
+ // And the second instance has to be usable afterwards, rather than left holding whatever the
+ // rejected attempt put there.
+ object recovered = cw.GetOrCreateObjectForComInstance(secondRaw, CreateObjectFlags.None);
+
+ Assert.NotSame(owned, recovered);
+ Assert.Same(recovered, cw.GetOrCreateObjectForComInstance(secondRaw, CreateObjectFlags.None));
+
+ Assert.True(ComWrappers.TryGetComInstance(recovered, out IntPtr recoveredUnknown));
+ Marshal.Release(recoveredUnknown);
+
+ GC.KeepAlive(owned);
+ GC.KeepAlive(recovered);
+
+ Marshal.Release(secondRaw);
+ Marshal.Release(firstRaw);
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ static void CreateAndAbandon(ComWrappers cw, IntPtr comInstance)
+ {
+ _ = cw.GetOrCreateObjectForComInstance(comInstance, CreateObjectFlags.None);
+ }
+ }
+
+ // Hands every caller the one wrapper, so that only one native reference is taken however many
+ // threads race. Creating a wrapper per caller and abandoning the losers is not an option here:
+ // ITrackerObjectWrapper's finalizer fails the run if its tracker object is still connected, and
+ // the winner keeps it connected for as long as the test needs it.
+ private sealed class SharedTrackerComWrappers : TestComWrappers
+ {
+ private readonly Barrier _barrier;
+ private readonly object _lock = new();
+ private object? _proxy;
+
+ public SharedTrackerComWrappers(Barrier barrier) => _barrier = barrier;
+
+ /// How many callers reached , so a test can prove they all raced.
+ public int CreateObjectCount;
+
+ protected override object CreateObject(IntPtr externalComObject, CreateObjectFlags flags)
+ {
+ Interlocked.Increment(ref CreateObjectCount);
+
+ _barrier.SignalAndWait(TimeSpan.FromMinutes(1));
+
+ lock (_lock)
+ {
+ return _proxy ??= base.CreateObject(externalComObject, flags);
+ }
+ }
+ }
+
+ // The concurrent test above uses CreateObjectFlags.None, so the wrapper it builds is not a
+ // reference tracker one and putting it in the tracker handle cache does nothing. This runs the
+ // same race with tracker objects, where one thread publishes the entry and the rest pick its
+ // wrapper up, and then checks what that registration is for: the wrapper has to be walked, or
+ // the managed objects the native object is holding are not kept alive through a collection.
+ [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
+ [Fact]
+ public void ValidateCreateObjectConcurrentTrackerRegistration()
+ {
+ Console.WriteLine($"Running {nameof(ValidateCreateObjectConcurrentTrackerRegistration)}...");
+
+ const int ThreadCount = 8;
+
+ IntPtr trackerObjRaw = MockReferenceTrackerRuntime.CreateTrackerObject();
+
+ using var barrier = new Barrier(ThreadCount);
+
+ var cw = new SharedTrackerComWrappers(barrier);
+ var failures = new ConcurrentQueue();
+
+ object[] results = new object[ThreadCount];
+ var threads = new Thread[ThreadCount];
+
+ for (int t = 0; t < threads.Length; t++)
+ {
+ int index = t;
+
+ threads[t] = new Thread(() =>
+ {
+ try
+ {
+ results[index] = cw.GetOrCreateObjectForComInstance(trackerObjRaw, CreateObjectFlags.TrackerObject);
+ }
+ catch (Exception e)
+ {
+ failures.Enqueue(e);
+ }
+ })
+ { IsBackground = true, Name = $"ComWrappers tracker {index}" };
+
+ threads[t].Start();
+ }
+
+ foreach (Thread thread in threads)
+ {
+ Assert.True(thread.Join(TimeSpan.FromMinutes(2)), "A worker thread did not finish, which suggests a deadlock while racing to publish.");
+ }
+
+ Assert.Empty(failures);
+
+ // Every thread really did miss the cache, so they all raced to publish rather than most of
+ // them quietly finding an entry someone else had already added.
+ Assert.Equal(ThreadCount, cw.CreateObjectCount);
+
+ // Ownership has been transferred to the wrapper.
+ Marshal.Release(trackerObjRaw);
+
+ var trackerObj = (ITrackerObjectWrapper)results[0];
+
+ foreach (object result in results)
+ {
+ Assert.Same(trackerObj, result);
+ }
+
+ Assert.True(ComWrappers.TryGetComInstance(trackerObj, out IntPtr unknown));
+
+ Marshal.Release(unknown);
+
+ // Whichever thread ended up publishing it, the wrapper has to have made it into the tracker
+ // handle cache, because that is what the runtime walks. If it had not, the managed objects
+ // reachable only through the native object would not be reported and would not survive here.
+ var testWrapperIds = new List();
+
+ for (int i = 0; i < 1000; ++i)
+ {
+ IntPtr testWrapper = cw.GetOrCreateComInterfaceForObject(new Test(), CreateComInterfaceFlags.TrackerSupport);
+
+ testWrapperIds.Add(trackerObj.AddObjectRef(testWrapper));
+
+ Marshal.Release(testWrapper);
+ }
+
+ ForceGC();
+
+ Assert.True(testWrapperIds.Count <= Test.InstanceCount);
+
+ foreach (int id in testWrapperIds)
+ {
+ trackerObj.DropObjectRef(id);
+ }
+
+ testWrapperIds.Clear();
+
+ ForceGC();
+
+ GC.KeepAlive(trackerObj);
+ }
+
+ // Hands every caller the same object, and holds them all inside 'CreateObject' until they have
+ // all arrived, so that they are guaranteed to have missed the cache and to be racing to publish.
+ private sealed unsafe class SharedProxyComWrappers : ComWrappers
+ {
+ private readonly Barrier _barrier;
+
+ public SharedProxyComWrappers(Barrier barrier) => _barrier = barrier;
+
+ public object Proxy { get; } = new();
+
+ /// How many callers reached , so a test can prove they all raced.
+ public int CreateObjectCount;
+
+ protected override ComInterfaceEntry* ComputeVtables(object obj, CreateComInterfaceFlags flags, out int count)
+ {
+ count = 0;
+ return null;
+ }
+
+ protected override object CreateObject(IntPtr externalComObject, CreateObjectFlags flags)
+ {
+ Interlocked.Increment(ref CreateObjectCount);
+
+ // Bounded rather than infinite so that a failure shows up as a failed assertion on the
+ // count below rather than as a hung test run.
+ _barrier.SignalAndWait(TimeSpan.FromMinutes(1));
+
+ return Proxy;
+ }
+
+ protected override void ReleaseObjects(IEnumerable objects) => throw new NotImplementedException();
+ }
+
+ // Several threads can all miss the cache for one COM instance and all call 'CreateObject', and an
+ // implementation is free to hand each of them the same object. Only one of those threads publishes
+ // it and the rest release the wrapper they built, so this checks that losing that race leaves the
+ // object usable and still mapped to the COM instance it was created for.
+ [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
+ [Fact]
+ public void ValidateCreateObjectRaceReturningSameObject()
+ {
+ Console.WriteLine($"Running {nameof(ValidateCreateObjectRaceReturningSameObject)}...");
+
+ const int ThreadCount = 4;
+
+ IntPtr trackerObjRaw = MockReferenceTrackerRuntime.CreateTrackerObject();
+
+ // The cache is keyed on the identity IUnknown, which is what this round trip produces.
+ Assert.Equal(0, Marshal.QueryInterface(trackerObjRaw, IUnknownVtbl.IID_IUnknown, out IntPtr identity));
+
+ using var barrier = new Barrier(ThreadCount);
+
+ var cw = new SharedProxyComWrappers(barrier);
+ var failures = new ConcurrentQueue();
+
+ object[] results = new object[ThreadCount];
+ var threads = new Thread[ThreadCount];
+
+ for (int t = 0; t < threads.Length; t++)
+ {
+ int index = t;
+
+ threads[t] = new Thread(() =>
+ {
+ try
+ {
+ results[index] = cw.GetOrCreateObjectForComInstance(trackerObjRaw, CreateObjectFlags.None);
+ }
+ catch (Exception e)
+ {
+ failures.Enqueue(e);
+ }
+ })
+ { IsBackground = true, Name = $"ComWrappers shared proxy {index}" };
+
+ threads[t].Start();
+ }
+
+ foreach (Thread thread in threads)
+ {
+ Assert.True(thread.Join(TimeSpan.FromMinutes(2)), "A worker thread did not finish, which suggests a deadlock while racing to publish.");
+ }
+
+ Assert.Empty(failures);
+
+ // Every thread really did miss the cache and go through 'CreateObject', so they all raced to
+ // publish rather than most of them quietly finding an entry someone else had already added.
+ Assert.Equal(ThreadCount, cw.CreateObjectCount);
+
+ // Winning or losing the race, every thread is handed the one object 'CreateObject' returned.
+ foreach (object result in results)
+ {
+ Assert.Same(cw.Proxy, result);
+ }
+
+ // Losing the race releases a wrapper, and it has to be the loser's rather than the published
+ // one, so the object still has to round trip back to the instance it was created for.
+ Assert.True(ComWrappers.TryGetComInstance(cw.Proxy, out IntPtr unknown));
+ Assert.Equal(identity, unknown);
+ Marshal.Release(unknown);
+
+ // And the cache still has to hand out that same object afterwards.
+ Assert.Same(cw.Proxy, cw.GetOrCreateObjectForComInstance(trackerObjRaw, CreateObjectFlags.None));
+
+ Marshal.Release(identity);
+ Marshal.Release(trackerObjRaw);
+ }
+
+ // Hands every caller a distinct object and keeps all of them, so a test can inspect the ones that
+ // lost the race to publish. Callers are held on a barrier so they are all guaranteed to have missed
+ // the cache and to be racing.
+ private sealed unsafe class DistinctProxyComWrappers : ComWrappers
+ {
+ private readonly Barrier _barrier;
+
+ public DistinctProxyComWrappers(Barrier barrier) => _barrier = barrier;
+
+ public ConcurrentQueue Created { get; } = new();
+
+ protected override ComInterfaceEntry* ComputeVtables(object obj, CreateComInterfaceFlags flags, out int count)
+ {
+ count = 0;
+ return null;
+ }
+
+ protected override object CreateObject(IntPtr externalComObject, CreateObjectFlags flags)
+ {
+ _barrier.SignalAndWait(TimeSpan.FromMinutes(1));
+
+ object proxy = new();
+
+ Created.Enqueue(proxy);
+
+ return proxy;
+ }
+
+ protected override void ReleaseObjects(IEnumerable objects) => throw new NotImplementedException();
+ }
+
+ // Only one of the objects 'CreateObject' produces for a COM instance can be published, and the
+ // wrappers built for the others are released. Nothing may be left behind for those objects: an
+ // implementation is free to keep hold of everything it returned, and the ones that lost have to
+ // behave exactly like objects that were never handed to ComWrappers at all.
+ [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
+ [Fact]
+ public void ValidateCreateObjectRaceLeavesNothingBehindForLosers()
+ {
+ Console.WriteLine($"Running {nameof(ValidateCreateObjectRaceLeavesNothingBehindForLosers)}...");
+
+ const int ThreadCount = 4;
+
+ IntPtr trackerObjRaw = MockReferenceTrackerRuntime.CreateTrackerObject();
+
+ Assert.Equal(0, Marshal.QueryInterface(trackerObjRaw, IUnknownVtbl.IID_IUnknown, out IntPtr identity));
+
+ using var barrier = new Barrier(ThreadCount);
+
+ var cw = new DistinctProxyComWrappers(barrier);
+ var failures = new ConcurrentQueue();
+
+ object[] results = new object[ThreadCount];
+ var threads = new Thread[ThreadCount];
+
+ for (int t = 0; t < threads.Length; t++)
+ {
+ int index = t;
+
+ threads[t] = new Thread(() =>
+ {
+ try
+ {
+ results[index] = cw.GetOrCreateObjectForComInstance(trackerObjRaw, CreateObjectFlags.None);
+ }
+ catch (Exception e)
+ {
+ failures.Enqueue(e);
+ }
+ })
+ { IsBackground = true, Name = $"ComWrappers distinct proxy {index}" };
+
+ threads[t].Start();
+ }
+
+ foreach (Thread thread in threads)
+ {
+ Assert.True(thread.Join(TimeSpan.FromMinutes(2)), "A worker thread did not finish, which suggests a deadlock while racing to publish.");
+ }
+
+ Assert.Empty(failures);
+
+ Assert.Equal(ThreadCount, cw.Created.Count);
+
+ object winner = results[0];
+
+ // Everyone gets the one object that was published, whichever thread produced it.
+ foreach (object result in results)
+ {
+ Assert.Same(winner, result);
+ }
+
+ Assert.True(ComWrappers.TryGetComInstance(winner, out IntPtr winnerUnknown));
+ Assert.Equal(identity, winnerUnknown);
+ Marshal.Release(winnerUnknown);
+
+ int winners = 0;
+
+ foreach (object created in cw.Created)
+ {
+ if (ReferenceEquals(created, winner))
+ {
+ winners++;
+ continue;
+ }
+
+ // A loser must look like an ordinary managed object. If a wrapper were left registered for
+ // it, these would throw instead, because releasing a wrapper zeroes the COM pointer that
+ // both of these paths hand to 'Marshal.QueryInterface'.
+ Assert.False(ComWrappers.TryGetComInstance(created, out IntPtr loserUnknown));
+ Assert.Equal(IntPtr.Zero, loserUnknown);
+
+ _ = new WeakReference(created);
+ }
+
+ Assert.Equal(1, winners);
+
+ // And a loser must still be usable as the wrapper for some other COM instance, rather than
+ // being permanently associated with the one it lost the race for.
+ IntPtr otherObjRaw = MockReferenceTrackerRuntime.CreateTrackerObject();
+
+ foreach (object created in cw.Created)
+ {
+ if (ReferenceEquals(created, winner))
+ {
+ continue;
+ }
+
+ Assert.Same(created, cw.GetOrRegisterObjectForComInstance(otherObjRaw, CreateObjectFlags.None, created));
+ break;
+ }
+
+ Marshal.Release(otherObjRaw);
+ Marshal.Release(identity);
+ Marshal.Release(trackerObjRaw);
+ }
+
// Verify that if a GC nulls the contents of a weak GCHandle but has not yet
// run finializers to remove that GCHandle from the cache, the state of the system is valid.
[ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
@@ -575,6 +1308,53 @@ static void CreateObject(ComWrappers cw, IntPtr trackerObj)
}
}
+ // A dead cache entry is replaced in place by the next RCW created for the same COM instance, so for a
+ // while the entry under that key belongs to a different wrapper than the one that is about to finalize.
+ // The old wrapper must remove only the entry it published, which is why entries are identified by the
+ // handle itself rather than by what it points at, since by then both point at nothing.
+ [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
+ [Fact]
+ public void ValidateReplacedCacheEntrySurvivesOldWrapperCleanUp()
+ {
+ Console.WriteLine($"Running {nameof(ValidateReplacedCacheEntrySurvivesOldWrapperCleanUp)}...");
+
+ var cw = new TestComWrappers();
+
+ IntPtr trackerObjRaw = MockReferenceTrackerRuntime.CreateTrackerObject();
+
+ CreateAndAbandonWrapper(cw, trackerObjRaw);
+
+ // Collect without draining finalizers, so the entry goes dead while the wrapper that owns it has
+ // most likely not run its finalizer yet.
+ GC.Collect();
+
+ // This takes over the dead entry, replacing the handle stored under that key.
+ var replacement = (ITrackerObjectWrapper)cw.GetOrCreateObjectForComInstance(trackerObjRaw, CreateObjectFlags.None);
+ Assert.NotNull(replacement);
+
+ // Now let the abandoned wrapper finalize and release, which removes its own cache entry.
+ ForceGC();
+
+ // The replacement is still alive, so it has to still be cached.
+ var lookup = (ITrackerObjectWrapper)cw.GetOrCreateObjectForComInstance(trackerObjRaw, CreateObjectFlags.None);
+ Assert.Same(replacement, lookup);
+
+ // It also has to still resolve back to the COM instance it wraps.
+ Assert.True(ComWrappers.TryGetComInstance(replacement, out IntPtr unknown));
+ Marshal.Release(unknown);
+
+ GC.KeepAlive(replacement);
+
+ Marshal.Release(trackerObjRaw);
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ static void CreateAndAbandonWrapper(ComWrappers cw, IntPtr instance)
+ {
+ var obj = (ITrackerObjectWrapper)cw.GetOrCreateObjectForComInstance(instance, CreateObjectFlags.None);
+ Assert.NotNull(obj);
+ }
+ }
+
[MethodImpl(MethodImplOptions.NoInlining)]
[ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
[Fact]
@@ -774,6 +1554,18 @@ public void ValidatePrecreatedExternalWrapper()
{
cw.GetOrRegisterObjectForComInstance(trackerObjRaw2, CreateObjectFlags.None, nativeWrapper2);
});
+
+ // The rejected registration must not leave anything behind for that COM instance, so creating a
+ // wrapper for it now has to work and has to produce a usable object.
+ var recovered = (ITrackerObjectWrapper)cw.GetOrCreateObjectForComInstance(trackerObjRaw2, CreateObjectFlags.None);
+ Assert.NotNull(recovered);
+ Assert.NotEqual(nativeWrapper2, recovered);
+
+ // Asking again returns the same wrapper, which confirms the entry that was just created is the live
+ // one rather than something left over from the rejected attempt.
+ var recoveredAgain = (ITrackerObjectWrapper)cw.GetOrCreateObjectForComInstance(trackerObjRaw2, CreateObjectFlags.None);
+ Assert.Equal(recovered, recoveredAgain);
+
Marshal.Release(trackerObjRaw2);
// Validate passing null wrapper fails.
@@ -835,6 +1627,89 @@ static WeakReference CreateAndRegisterWrapper(ComWrappers
}
}
+ // Every other test in this file uses an RCW type that has a finalizer, and a wrapper allocates a
+ // second GC handle for those. An RCW with no finalizer gets a single handle that tracks
+ // resurrection instead, and that is the handle the cache holds, so its entries go dead at a
+ // different point in a collection than the ones covered above.
+ [ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
+ [Fact]
+ public void ValidateExternalWrapperCacheCleanUpWithoutFinalizer()
+ {
+ Console.WriteLine($"Running {nameof(ValidateExternalWrapperCacheCleanUpWithoutFinalizer)}...");
+
+ var cw = new TestComWrappers()
+ {
+ UseManualReleaseITestObjectWrapper = true,
+ };
+
+ var test = new Test();
+
+ IntPtr comWrapper = cw.GetOrCreateComInterfaceForObject(test, CreateComInterfaceFlags.None);
+
+ Assert.NotEqual(IntPtr.Zero, comWrapper);
+
+ WeakReference first = CreateAndAbandonWrapper(cw, comWrapper);
+
+ // Collect without draining finalizers, so the entry is dead but the wrapper that owns it has
+ // not run yet and so has not removed it. Creating again has to see through the dead entry.
+ GC.Collect();
+
+ // This RCW type has no finalizer, so a single collection is enough for it to be gone. That is
+ // the property this test exists for: such an RCW gets one handle that tracks resurrection, and
+ // the cache holds that handle, so the entry dies here rather than after finalization.
+ Assert.False(first.TryGetTarget(out _));
+
+ WeakReference second = CreateAndAbandonWrapper(cw, comWrapper);
+
+ // The dead entry did not prevent a new RCW being created and cached for the same instance.
+ // Checked in a separate frame so that the strong reference it needs does not outlive it and
+ // keep the RCW alive through the collection below.
+ AssertUsableAndDrop(second);
+
+ // Now let the wrapper finalizers run, which is what removes entries, and check the cache is
+ // still able to hand out a working RCW afterwards.
+ ForceGC();
+
+ Assert.False(second.TryGetTarget(out _));
+
+ var third = (ManualReleaseITestObjectWrapper)cw.GetOrCreateObjectForComInstance(comWrapper, CreateObjectFlags.None);
+
+ Assert.True(ComWrappers.TryGetComInstance(third, out IntPtr unknown));
+
+ Assert.Equal(0, Marshal.QueryInterface(comWrapper, IUnknownVtbl.IID_IUnknown, out IntPtr identity));
+ Assert.Equal(identity, unknown);
+
+ Marshal.Release(identity);
+ Marshal.Release(unknown);
+
+ third.FinalRelease();
+
+ GC.KeepAlive(test);
+
+ Marshal.Release(comWrapper);
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ static void AssertUsableAndDrop(WeakReference reference)
+ {
+ Assert.True(reference.TryGetTarget(out object? target));
+ Assert.True(ComWrappers.TryGetComInstance(target, out IntPtr unknown));
+
+ Marshal.Release(unknown);
+ }
+
+ // This wrapper type releases the interface pointer its constructor took by hand rather than
+ // from a finalizer, so it is released here before the wrapper is dropped.
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ static WeakReference CreateAndAbandonWrapper(ComWrappers cw, IntPtr comWrapper)
+ {
+ var wrapper = (ManualReleaseITestObjectWrapper)cw.GetOrCreateObjectForComInstance(comWrapper, CreateObjectFlags.None);
+
+ wrapper.FinalRelease();
+
+ return new WeakReference(wrapper);
+ }
+ }
+
[ActiveIssue("Not supported on Mono", TestRuntimes.Mono)]
[Fact]
public void ValidateSuppliedInnerNotAggregation()