Skip to content

Commit

Permalink
Add support for ComWrappers-based RCWs to the special native WeakRefe…
Browse files Browse the repository at this point in the history
…rence support. (#35819)

* Add support for ComWrappers-based RCWs to the special native WeakReference support.

Rename the WinRT weak reference handle to "native COM weak reference" handle.

* Fix line endings.

* Fix line endings (try 2).

* Revert change to prebuilt idl

* Try updating prebuilt again.

* Remove accidental duplicate of the test.

* PR feedback.

* PR Feedback.

* React to global ComWrappers changes.

* Add back WinRT enum member to prebuilt idl.

* Add back old enum member to idl for backcompat.

* Change definition of enum member to explicitly assign the same value.

* Code cleanup and go down a non-allocating route when possible.

* Switch to preemptive mode for the QI call.

* Fix contracts

* Add a check before calling GetComWeakReference so we only call it when the object has interop info attached.

* Apply early check to WeakReference<T> as well.

* Make sure we only make one call to PassiveGetSyncBlock instead of 2.

* PR Feedback.

* ComWrappersNative::GetIdentityForObject can trigger GC since we transition to and from pre-emptive when calling into the external QI.
  • Loading branch information
jkoritzinsky committed May 8, 2020
1 parent 4804b8d commit 16b7db2
Show file tree
Hide file tree
Showing 26 changed files with 587 additions and 122 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,15 @@ public static void RegisterForTrackerSupport(ComWrappers instance)
{
throw new InvalidOperationException(SR.InvalidOperation_ResetGlobalComWrappersInstance);
}

SetGlobalInstanceRegisteredForTrackerSupport();
}


[DllImport(RuntimeHelpers.QCall)]
[SuppressGCTransition]
private static extern void SetGlobalInstanceRegisteredForTrackerSupport();

/// <summary>
/// Register a <see cref="ComWrappers" /> instance to be used as the global instance for marshalling in the runtime.
/// </summary>
Expand Down Expand Up @@ -390,4 +397,4 @@ internal static int CallICustomQueryInterface(object customQueryInterfaceMaybe,
return (int)customQueryInterface.GetInterface(ref iid, out ppObject);
}
}
}
}
2 changes: 1 addition & 1 deletion src/coreclr/src/debug/daccess/daccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8252,7 +8252,7 @@ void CALLBACK DacHandleWalker::EnumCallbackSOS(PTR_UNCHECKED_OBJECTREF handle, u
if (param->Type == HNDTYPE_DEPENDENT)
data.Secondary = GetDependentHandleSecondary(handle.GetAddr()).GetAddr();
#ifdef FEATURE_COMINTEROP
else if (param->Type == HNDTYPE_WEAK_WINRT)
else if (param->Type == HNDTYPE_WEAK_NATIVE_COM)
data.Secondary = HndGetHandleExtraInfo(handle.GetAddr());
#endif // FEATURE_COMINTEROP
else
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/src/debug/daccess/dacdbiimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7491,8 +7491,8 @@ UINT32 DacRefWalker::GetHandleWalkerMask()
if ((mHandleMask & CorHandleWeakRefCount) || (mHandleMask & CorHandleStrongRefCount))
result |= (1 << HNDTYPE_REFCOUNTED);

if (mHandleMask & CorHandleWeakWinRT)
result |= (1 << HNDTYPE_WEAK_WINRT);
if (mHandleMask & CorHandleWeakNativeCom)
result |= (1 << HNDTYPE_WEAK_NATIVE_COM);
#endif // FEATURE_COMINTEROP

if (mHandleMask & CorHandleStrongDependent)
Expand Down Expand Up @@ -7667,8 +7667,8 @@ void CALLBACK DacHandleWalker::EnumCallbackDac(PTR_UNCHECKED_OBJECTREF handle, u
data.i64ExtraData = refCnt;
break;

case HNDTYPE_WEAK_WINRT:
data.dwType = (DWORD)CorHandleWeakWinRT;
case HNDTYPE_WEAK_NATIVE_COM:
data.dwType = (DWORD)CorHandleWeakNativeCom;
break;
#endif

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/src/debug/daccess/request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3253,7 +3253,7 @@ HRESULT ClrDataAccess::GetHandleEnum(ISOSHandleEnum **ppHandleEnum)
unsigned int types[] = {HNDTYPE_WEAK_SHORT, HNDTYPE_WEAK_LONG, HNDTYPE_STRONG, HNDTYPE_PINNED, HNDTYPE_VARIABLE, HNDTYPE_DEPENDENT,
HNDTYPE_ASYNCPINNED, HNDTYPE_SIZEDREF,
#ifdef FEATURE_COMINTEROP
HNDTYPE_REFCOUNTED, HNDTYPE_WEAK_WINRT
HNDTYPE_REFCOUNTED, HNDTYPE_WEAK_NATIVE_COM
#endif
};

Expand Down Expand Up @@ -3291,7 +3291,7 @@ HRESULT ClrDataAccess::GetHandleEnumForGC(unsigned int gen, ISOSHandleEnum **ppH
unsigned int types[] = {HNDTYPE_WEAK_SHORT, HNDTYPE_WEAK_LONG, HNDTYPE_STRONG, HNDTYPE_PINNED, HNDTYPE_VARIABLE, HNDTYPE_DEPENDENT,
HNDTYPE_ASYNCPINNED, HNDTYPE_SIZEDREF,
#ifdef FEATURE_COMINTEROP
HNDTYPE_REFCOUNTED, HNDTYPE_WEAK_WINRT
HNDTYPE_REFCOUNTED, HNDTYPE_WEAK_NATIVE_COM
#endif
};

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/dlls/mscorrc/mscorrc.rc
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ BEGIN
IDS_EE_WINRT_NOT_FACTORY_FOR_TYPE "Windows Runtime factory '%1' is not a factory for Windows Runtime type '%2'."
IDS_EE_WINRT_INVALID_FACTORY_FOR_TYPE "Windows Runtime type '%1' has a invalid Windows Runtime factory"
IDS_EE_CANNOTCAST_NOMARSHAL "The Windows Runtime Object can only be used in the threading context where it was created, because it implements INoMarshal or has MarshalingBehaviorAttribute(MarshalingType.None) set."
IDS_EE_WINRT_WEAKREF_BAD_TYPE "The object resolved by a native IWeakReference has an incompatible type for its managed WeakReference instance.\r\nExpected WeakReference target type: '%1'\r\nNative IWeakReference returned type: '%2'"
IDS_EE_NATIVE_COM_WEAKREF_BAD_TYPE "The object resolved by a native IWeakReference has an incompatible type for its managed WeakReference instance.\r\nExpected WeakReference target type: '%1'\r\nNative IWeakReference returned type: '%2'"
#endif // FEATURE_COMINTEROP

IDS_EE_INTEROP_CODE_SIZE_COMMENT "Code size"
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/dlls/mscorrc/resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@


#ifdef FEATURE_COMINTEROP
#define IDS_EE_WINRT_WEAKREF_BAD_TYPE 0x262e
#define IDS_EE_NATIVE_COM_WEAKREF_BAD_TYPE 0x262e
#endif // FEATURE_COMINTEROP

#define IDS_EE_BADMARSHAL_TYPE_ANSIBSTR 0x262f
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/gc/gchandletable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ Object* GCHandleManager::InterlockedCompareExchangeObjectInHandle(OBJECTHANDLE h
HandleType GCHandleManager::HandleFetchType(OBJECTHANDLE handle)
{
uint32_t type = ::HandleFetchType(handle);
assert(type >= HNDTYPE_WEAK_SHORT && type <= HNDTYPE_WEAK_WINRT);
assert(type >= HNDTYPE_WEAK_SHORT && type <= HNDTYPE_WEAK_NATIVE_COM);
return static_cast<HandleType>(type);
}

Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/src/gc/gcinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -422,18 +422,18 @@ typedef enum
HNDTYPE_SIZEDREF = 8,

/*
* WINRT WEAK HANDLES
* NATIVE WEAK HANDLES
*
* WinRT weak reference handles hold two different types of weak handles to any
* Native weak reference handles hold two different types of weak handles to any
* RCW with an underlying COM object that implements IWeakReferenceSource. The
* object reference itself is a short weak handle to the RCW. In addition an
* IWeakReference* to the underlying COM object is stored, allowing the handle
* to create a new RCW if the existing RCW is collected. This ensures that any
* code holding onto a WinRT weak reference can always access an RCW to the
* code holding onto a native weak reference can always access an RCW to the
* underlying COM object as long as it has not been released by all of its strong
* references.
*/
HNDTYPE_WEAK_WINRT = 9
HNDTYPE_WEAK_NATIVE_COM = 9
} HandleType;

typedef enum
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/src/gc/handletable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -409,10 +409,10 @@ void HndDestroyHandleOfUnknownType(HHANDLETABLE hTable, OBJECTHANDLE handle)
_ASSERTE(handle);

#ifdef FEATURE_COMINTEROP
// If we're being asked to destroy a WinRT weak handle, that will cause a leak
// If we're being asked to destroy a native COM weak handle, that will cause a leak
// of the IWeakReference* that it holds in its extra data. Instead of using this
// API use DestroyWinRTWeakHandle instead.
_ASSERTE(HandleFetchType(handle) != HNDTYPE_WEAK_WINRT);
// API use DestroyNativeComWeakHandle instead.
_ASSERTE(HandleFetchType(handle) != HNDTYPE_WEAK_NATIVE_COM);
#endif // FEATURE_COMINTEROP

// fetch the type and then free normally
Expand Down
16 changes: 8 additions & 8 deletions src/coreclr/src/gc/objecthandle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ void CALLBACK ScanPointerForProfilerAndETW(_UNCHECKED_OBJECTREF *pObjRef, uintpt
case HNDTYPE_WEAK_SHORT:
case HNDTYPE_WEAK_LONG:
#ifdef FEATURE_COMINTEROP
case HNDTYPE_WEAK_WINRT:
case HNDTYPE_WEAK_NATIVE_COM:
#endif // FEATURE_COMINTEROP
rootFlags |= kEtwGCRootFlagsWeakRef;
break;
Expand Down Expand Up @@ -520,7 +520,7 @@ static const uint32_t s_rgTypeFlags[] =
HNDF_EXTRAINFO, // HNDTYPE_DEPENDENT
HNDF_NORMAL, // HNDTYPE_ASYNCPINNED
HNDF_EXTRAINFO, // HNDTYPE_SIZEDREF
HNDF_EXTRAINFO, // HNDTYPE_WEAK_WINRT
HNDF_EXTRAINFO, // HNDTYPE_WEAK_NATIVE_COM
};

int getNumberOfSlots()
Expand Down Expand Up @@ -1380,7 +1380,7 @@ void Ref_CheckAlive(uint32_t condemned, uint32_t maxgen, uintptr_t lp1)
{
HNDTYPE_WEAK_SHORT
#ifdef FEATURE_COMINTEROP
, HNDTYPE_WEAK_WINRT
, HNDTYPE_WEAK_NATIVE_COM
#endif // FEATURE_COMINTEROP
};
uint32_t flags = (((ScanContext*) lp1)->concurrent) ? HNDGCF_ASYNC : HNDGCF_NORMAL;
Expand Down Expand Up @@ -1439,7 +1439,7 @@ void Ref_UpdatePointers(uint32_t condemned, uint32_t maxgen, ScanContext* sc, Re
HNDTYPE_REFCOUNTED,
#endif // FEATURE_COMINTEROP || FEATURE_REDHAWK
#ifdef FEATURE_COMINTEROP
HNDTYPE_WEAK_WINRT,
HNDTYPE_WEAK_NATIVE_COM,
#endif // FEATURE_COMINTEROP
HNDTYPE_SIZEDREF,
};
Expand Down Expand Up @@ -1485,7 +1485,7 @@ void Ref_ScanHandlesForProfilerAndETW(uint32_t maxgen, uintptr_t lp1, handle_sca
HNDTYPE_REFCOUNTED,
#endif // FEATURE_COMINTEROP || FEATURE_REDHAWK
#ifdef FEATURE_COMINTEROP
HNDTYPE_WEAK_WINRT,
HNDTYPE_WEAK_NATIVE_COM,
#endif // FEATURE_COMINTEROP
HNDTYPE_PINNED,
// HNDTYPE_VARIABLE,
Expand Down Expand Up @@ -1631,7 +1631,7 @@ void Ref_AgeHandles(uint32_t condemned, uint32_t maxgen, uintptr_t lp1)
HNDTYPE_REFCOUNTED,
#endif // FEATURE_COMINTEROP || FEATURE_REDHAWK
#ifdef FEATURE_COMINTEROP
HNDTYPE_WEAK_WINRT,
HNDTYPE_WEAK_NATIVE_COM,
#endif // FEATURE_COMINTEROP
HNDTYPE_ASYNCPINNED,
HNDTYPE_SIZEDREF,
Expand Down Expand Up @@ -1674,7 +1674,7 @@ void Ref_RejuvenateHandles(uint32_t condemned, uint32_t maxgen, uintptr_t lp1)
HNDTYPE_REFCOUNTED,
#endif // FEATURE_COMINTEROP || FEATURE_REDHAWK
#ifdef FEATURE_COMINTEROP
HNDTYPE_WEAK_WINRT,
HNDTYPE_WEAK_NATIVE_COM,
#endif // FEATURE_COMINTEROP
HNDTYPE_ASYNCPINNED,
HNDTYPE_SIZEDREF,
Expand Down Expand Up @@ -1716,7 +1716,7 @@ void Ref_VerifyHandleTable(uint32_t condemned, uint32_t maxgen, ScanContext* sc)
HNDTYPE_REFCOUNTED,
#endif // FEATURE_COMINTEROP || FEATURE_REDHAWK
#ifdef FEATURE_COMINTEROP
HNDTYPE_WEAK_WINRT,
HNDTYPE_WEAK_NATIVE_COM,
#endif // FEATURE_COMINTEROP
HNDTYPE_ASYNCPINNED,
HNDTYPE_SIZEDREF,
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/src/inc/cordebug.idl
Original file line number Diff line number Diff line change
Expand Up @@ -2567,7 +2567,8 @@ typedef enum CorGCReferenceType
CorHandleStrongDependent = 1<<6,
CorHandleStrongAsyncPinned = 1<<7,
CorHandleStrongSizedByref = 1<<8,
CorHandleWeakWinRT = 1<<9,
CorHandleWeakNativeCom = 1<<9,
CorHandleWeakWinRT = CorHandleWeakNativeCom,

CorReferenceStack = 0x80000001,
CorReferenceFinalizer = 80000002,
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/src/pal/prebuilt/inc/cordebug.h
Original file line number Diff line number Diff line change
Expand Up @@ -6376,7 +6376,8 @@ enum CorGCReferenceType
CorHandleStrongDependent = ( 1 << 6 ) ,
CorHandleStrongAsyncPinned = ( 1 << 7 ) ,
CorHandleStrongSizedByref = ( 1 << 8 ) ,
CorHandleWeakWinRT = ( 1 << 9 ) ,
CorHandleWeakNativeCom = ( 1 << 9 ) ,
CorHandleWeakWinRT = CorHandleWeakNativeCom,
CorReferenceStack = 0x80000001,
CorReferenceFinalizer = 80000002,
CorHandleStrongOnly = 0x1e3,
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/src/vm/appdomain.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1110,10 +1110,10 @@ class BaseDomain
return ::CreateRefcountedHandle(m_handleStore, object);
}

OBJECTHANDLE CreateWinRTWeakHandle(OBJECTREF object, IWeakReference* pWinRTWeakReference)
OBJECTHANDLE CreateNativeComWeakHandle(OBJECTREF object, IWeakReference* pComWeakReference)
{
WRAPPER_NO_CONTRACT;
return ::CreateWinRTWeakHandle(m_handleStore, object, pWinRTWeakReference);
return ::CreateNativeComWeakHandle(m_handleStore, object, pComWeakReference);
}
#endif // FEATURE_COMINTEROP

Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/vm/ecalllist.h
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,7 @@ FCFuncStart(gComWrappersFuncs)
QCFuncElement("TryGetOrCreateComInterfaceForObjectInternal", ComWrappersNative::TryGetOrCreateComInterfaceForObject)
QCFuncElement("TryGetOrCreateObjectForComInstanceInternal", ComWrappersNative::TryGetOrCreateObjectForComInstance)
QCFuncElement("SetGlobalInstanceRegisteredForMarshalling", GlobalComWrappersForMarshalling::SetGlobalInstanceRegisteredForMarshalling)
QCFuncElement("SetGlobalInstanceRegisteredForTrackerSupport", GlobalComWrappersForTrackerSupport::SetGlobalInstanceRegisteredForTrackerSupport)
FCFuncEnd()
#endif // FEATURE_COMWRAPPERS

Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/src/vm/gchandleutilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ inline OBJECTHANDLE CreateGlobalRefcountedHandle(OBJECTREF object)
// Special handle creation convenience functions

#ifdef FEATURE_COMINTEROP
inline OBJECTHANDLE CreateWinRTWeakHandle(IGCHandleStore* store, OBJECTREF object, IWeakReference* pWinRTWeakReference)
inline OBJECTHANDLE CreateNativeComWeakHandle(IGCHandleStore* store, OBJECTREF object, IWeakReference* pComWeakReference)
{
OBJECTHANDLE hnd = store->CreateHandleWithExtraInfo(OBJECTREFToObject(object), HNDTYPE_WEAK_WINRT, (void*)pWinRTWeakReference);
OBJECTHANDLE hnd = store->CreateHandleWithExtraInfo(OBJECTREFToObject(object), HNDTYPE_WEAK_NATIVE_COM, (void*)pComWeakReference);
if (!hnd)
{
COMPlusThrowOM();
Expand Down Expand Up @@ -363,7 +363,7 @@ inline void DestroyTypedHandle(OBJECTHANDLE handle)
}

#ifdef FEATURE_COMINTEROP
inline void DestroyWinRTWeakHandle(OBJECTHANDLE handle)
inline void DestroyNativeComWeakHandle(OBJECTHANDLE handle)
{
CONTRACTL
{
Expand All @@ -375,7 +375,7 @@ inline void DestroyWinRTWeakHandle(OBJECTHANDLE handle)
CONTRACTL_END;

// Release the WinRT weak reference if we have one. We're assuming that this will not reenter the
// runtime, since if we are pointing at a managed object, we should not be using HNDTYPE_WEAK_WINRT
// runtime, since if we are pointing at a managed object, we should not be using HNDTYPE_WEAK_NATIVE_COM
// but rather HNDTYPE_WEAK_SHORT or HNDTYPE_WEAK_LONG.
void* pExtraInfo = GCHandleUtilities::GetGCHandleManager()->GetExtraInfoFromHandle(handle);
IWeakReference* pWinRTWeakReference = reinterpret_cast<IWeakReference*>(pExtraInfo);
Expand All @@ -385,7 +385,7 @@ inline void DestroyWinRTWeakHandle(OBJECTHANDLE handle)
}

DiagHandleDestroyed(handle);
GCHandleUtilities::GetGCHandleManager()->DestroyHandleOfType(handle, HNDTYPE_WEAK_WINRT);
GCHandleUtilities::GetGCHandleManager()->DestroyHandleOfType(handle, HNDTYPE_WEAK_NATIVE_COM);
}
#endif

Expand Down
Loading

0 comments on commit 16b7db2

Please sign in to comment.