Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 16 commits into from
Jan 7, 2025
+409 −154
Merged

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

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4a1e2e8
Exit the lock before we call into user code and handle losing the rac…
jkoritzinsky Dec 18, 2024
b59f2f7
Better encapsulate logic for the RCW table and the RCW cache.
jkoritzinsky Dec 20, 2024
98fea75
Flip the order of operations to match CoreCLR:
jkoritzinsky Dec 20, 2024
493a0ac
Remove outdated comment
jkoritzinsky Dec 20, 2024
c1994ed
Like CoreCLR, record the NativeObjectWrapper (CoreCLR's ExtObjCxt) in…
jkoritzinsky Dec 20, 2024
b457f63
Extract the RCW cache to its own type and do some more cleanup.
jkoritzinsky Dec 20, 2024
d78d870
Pass the NativeObjectWrapper and comProxy as a pair and ensure that t…
jkoritzinsky Dec 20, 2024
0e9dd87
Reorganize code to reduce indentation
jkoritzinsky Dec 20, 2024
f1e41f8
A little more refactoring
jkoritzinsky Dec 20, 2024
9a0f7a6
Rename based on our naming conventions.
jkoritzinsky Dec 21, 2024
068b913
Fix UniqueInstance flag
jkoritzinsky Dec 23, 2024
d5fa981
Add a test for the deadlock
jkoritzinsky Dec 23, 2024
ebdead5
Use manually-managed weak GCHandles to reduce allocations.
jkoritzinsky Dec 23, 2024
40b994e
PR feedback
jkoritzinsky Dec 27, 2024
b46e3a8
Add a cross-apartment test (Windows-only obviously)
jkoritzinsky Jan 6, 2025
9cc64e7
PR feedback
jkoritzinsky Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 41 additions & 19 deletions ...iveaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -968,21 +968,40 @@ private unsafe bool TryGetOrCreateObjectForComInstanceInternal(
_rcwCache.Remove(identity);
}
}
}

if (wrapperMaybe is not null)
{
retValue = wrapperMaybe;

if (wrapperMaybe is not null)
using (_lock.EnterScope())
{
retValue = wrapperMaybe;
NativeObjectWrapper wrapper = NativeObjectWrapper.Create(
identity,
inner,
this,
retValue,
flags);
if (!s_rcwTable.TryAdd(retValue, wrapper))
if (!s_rcwTable.TryGetValue(retValue, out NativeObjectWrapper? wrapper))
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
{
wrapper.Release();
throw new NotSupportedException();
// Exit the lock before we create the native object wrapper
// as we may execute arbitrary user code.
_lock.Exit();
try
{
NativeObjectWrapper newWrapper = NativeObjectWrapper.Create(
identity,
inner,
this,
retValue,
flags);

wrapper = s_rcwTable.GetValue(retValue, _ => newWrapper);
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
if (wrapper != newWrapper)
{
newWrapper.Release();
}
}
finally
{
_lock.Enter();
}
}

_rcwCache.Add(identity, wrapper._proxyHandle);
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
AddWrapperToReferenceTrackerHandleCache(wrapper);
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
return true;
Expand Down Expand Up @@ -1026,16 +1045,17 @@ private unsafe bool TryGetOrCreateObjectForComInstanceInternal(

if (flags.HasFlag(CreateObjectFlags.UniqueInstance))
{
NativeObjectWrapper wrapper = NativeObjectWrapper.Create(
NativeObjectWrapper newWrapper = NativeObjectWrapper.Create(
identity,
inner,
null, // No need to cache NativeObjectWrapper for unique instances. They are not cached.
retValue,
flags);
if (!s_rcwTable.TryAdd(retValue, wrapper))

NativeObjectWrapper wrapper = s_rcwTable.GetValue(retValue, _ => newWrapper);
if (wrapper != newWrapper)
{
wrapper.Release();
throw new NotSupportedException();
newWrapper.Release();
}
AddWrapperToReferenceTrackerHandleCache(wrapper);
return true;
Expand All @@ -1061,17 +1081,19 @@ private unsafe bool TryGetOrCreateObjectForComInstanceInternal(
}
else
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
{
NativeObjectWrapper wrapper = NativeObjectWrapper.Create(
NativeObjectWrapper newWrapper = NativeObjectWrapper.Create(
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
identity,
inner,
this,
retValue,
flags);
if (!s_rcwTable.TryAdd(retValue, wrapper))

NativeObjectWrapper wrapper = s_rcwTable.GetValue(retValue, _ => newWrapper);
if (wrapper != newWrapper)
{
wrapper.Release();
throw new NotSupportedException();
newWrapper.Release();
}

_rcwCache.Add(identity, wrapper._proxyHandle);
AddWrapperToReferenceTrackerHandleCache(wrapper);
}
Expand Down
Loading