Skip to content

Commit

Permalink
Switch to Try semantics
Browse files Browse the repository at this point in the history
  • Loading branch information
elinor-fung committed Mar 12, 2020
1 parent 0a3ddb4 commit dad304e
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ private struct ComInterfaceInstance
/// <returns>The generated COM interface that can be passed outside the .NET runtime.</returns>
public IntPtr GetOrCreateComInterfaceForObject(object instance, CreateComInterfaceFlags flags)
{
IntPtr ptr = GetOrCreateComInterfaceForObjectInternal(this, instance, flags);
if (ptr == IntPtr.Zero)
IntPtr ptr;
if (!TryGetOrCreateComInterfaceForObjectInternal(this, instance, flags, out ptr))
throw new ArgumentException();

return ptr;
Expand All @@ -132,20 +132,21 @@ public IntPtr GetOrCreateComInterfaceForObject(object instance, CreateComInterfa
/// <param name="impl">The <see cref="ComWrappers" /> implemenentation to use when creating the COM representation.</param>
/// <param name="instance">The managed object to expose outside the .NET runtime.</param>
/// <param name="flags">Flags used to configure the generated interface.</param>
/// <returns>The generated COM interface that can be passed outside the .NET runtime or IntPtr.Zero if it could not be created.</returns>
/// <param name="retValue">The generated COM interface that can be passed outside the .NET runtime or IntPtr.Zero if it could not be created.</param>
/// <returns>Returns <c>true</c> if a CEM representation could be created, <c>false</c> otherwise</returns>
/// <remarks>
/// If <paramref name="impl" /> is <c>null</c>, the global instance (if registered) will be used.
/// </remarks>
internal static IntPtr GetOrCreateComInterfaceForObjectInternal(ComWrappers? impl, object instance, CreateComInterfaceFlags flags)
internal static bool TryGetOrCreateComInterfaceForObjectInternal(ComWrappers? impl, object instance, CreateComInterfaceFlags flags, out IntPtr retValue)
{
if (instance == null)
throw new ArgumentNullException(nameof(instance));

return GetOrCreateComInterfaceForObjectInternal(ObjectHandleOnStack.Create(ref impl), ObjectHandleOnStack.Create(ref instance), flags);
return TryGetOrCreateComInterfaceForObjectInternal(ObjectHandleOnStack.Create(ref impl), ObjectHandleOnStack.Create(ref instance), flags, out retValue);
}

[DllImport(RuntimeHelpers.QCall)]
private static extern IntPtr GetOrCreateComInterfaceForObjectInternal(ObjectHandleOnStack comWrappersImpl, ObjectHandleOnStack instance, CreateComInterfaceFlags flags);
private static extern bool TryGetOrCreateComInterfaceForObjectInternal(ObjectHandleOnStack comWrappersImpl, ObjectHandleOnStack instance, CreateComInterfaceFlags flags, out IntPtr retValue);

/// <summary>
/// Compute the desired Vtable for <paramref name="obj"/> respecting the values of <paramref name="flags"/>.
Expand Down Expand Up @@ -184,11 +185,11 @@ internal static IntPtr GetOrCreateComInterfaceForObjectInternal(ComWrappers? imp
/// <returns>Returns a managed object associated with the supplied external COM object.</returns>
public object GetOrCreateObjectForComInstance(IntPtr externalComObject, CreateObjectFlags flags)
{
object? obj = GetOrCreateObjectForComInstanceInternal(this, externalComObject, flags, null);
if (obj == null)
object? obj;
if (!TryGetOrCreateObjectForComInstanceInternal(this, externalComObject, flags, null, out obj))
throw new ArgumentNullException();

return obj;
return obj!;
}

/// <summary>
Expand Down Expand Up @@ -227,11 +228,11 @@ public object GetOrRegisterObjectForComInstance(IntPtr externalComObject, Create
if (wrapper == null)
throw new ArgumentNullException(nameof(externalComObject));

object? obj = GetOrCreateObjectForComInstanceInternal(this, externalComObject, flags, wrapper);
if (obj == null)
object? obj;
if (!TryGetOrCreateObjectForComInstanceInternal(this, externalComObject, flags, wrapper, out obj))
throw new ArgumentNullException();

return obj;
return obj!;
}

/// <summary>
Expand All @@ -241,24 +242,23 @@ public object GetOrRegisterObjectForComInstance(IntPtr externalComObject, Create
/// <param name="externalComObject">Object to import for usage into the .NET runtime.</param>
/// <param name="flags">Flags used to describe the external object.</param>
/// <param name="wrapperMaybe">The <see cref="object"/> to be used as the wrapper for the external object.</param>
/// <returns>Returns a managed object associated with the supplied external COM object or <c>null</c> if it could not be created.</returns>
/// <param name="retValue">The managed object associated with the supplied external COM object or <c>null</c> if it could not be created.</param>
/// <returns>Returns <c>true</c> if a managed object could be retrieved/created, <c>false</c> otherwise</returns>
/// <remarks>
/// If <paramref name="impl" /> is <c>null</c>, the global instance (if registered) will be used.
/// </remarks>
internal static object? GetOrCreateObjectForComInstanceInternal(ComWrappers? impl, IntPtr externalComObject, CreateObjectFlags flags, object? wrapperMaybe)
internal static bool TryGetOrCreateObjectForComInstanceInternal(ComWrappers? impl, IntPtr externalComObject, CreateObjectFlags flags, object? wrapperMaybe, out object? retValue)
{
if (externalComObject == IntPtr.Zero)
throw new ArgumentNullException(nameof(externalComObject));

object? wrapperMaybeLocal = wrapperMaybe;
object? retValue = null;
GetOrCreateObjectForComInstanceInternal(ObjectHandleOnStack.Create(ref impl), externalComObject, flags, ObjectHandleOnStack.Create(ref wrapperMaybeLocal), ObjectHandleOnStack.Create(ref retValue));

return retValue;
retValue = null;
return TryGetOrCreateObjectForComInstanceInternal(ObjectHandleOnStack.Create(ref impl), externalComObject, flags, ObjectHandleOnStack.Create(ref wrapperMaybeLocal), ObjectHandleOnStack.Create(ref retValue));
}

[DllImport(RuntimeHelpers.QCall)]
private static extern void GetOrCreateObjectForComInstanceInternal(ObjectHandleOnStack comWrappersImpl, IntPtr externalComObject, CreateObjectFlags flags, ObjectHandleOnStack wrapper, ObjectHandleOnStack retValue);
private static extern bool TryGetOrCreateObjectForComInstanceInternal(ObjectHandleOnStack comWrappersImpl, IntPtr externalComObject, CreateObjectFlags flags, ObjectHandleOnStack wrapper, ObjectHandleOnStack retValue);

/// <summary>
/// Called when a request is made for a collection of objects to be released outside of normal object or COM interface lifetime.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,8 @@ public static string GetTypeInfoName(ITypeInfo typeInfo)
}

// Passing null as the ComWrapper implementation will use the globally registered wrappper (if available)
IntPtr ptrMaybe = ComWrappers.GetOrCreateComInterfaceForObjectInternal(impl: null, o, CreateComInterfaceFlags.TrackerSupport);
if (ptrMaybe != IntPtr.Zero)
IntPtr ptrMaybe;
if (ComWrappers.TryGetOrCreateComInterfaceForObjectInternal(impl: null, o, CreateComInterfaceFlags.TrackerSupport, out ptrMaybe))
return ptrMaybe;

return GetIUnknownForObjectNative(o, false);
Expand Down Expand Up @@ -421,9 +421,9 @@ public static object GetObjectForIUnknown(IntPtr /* IUnknown* */ pUnk)
}

// Passing null as the ComWrapper implementation will use the globally registered wrappper (if available)
object? objMaybe = ComWrappers.GetOrCreateObjectForComInstanceInternal(impl: null, pUnk, CreateObjectFlags.TrackerObject, wrapperMaybe: null);
if (objMaybe != null)
return objMaybe;
object? objMaybe;
if (ComWrappers.TryGetOrCreateObjectForComInstanceInternal(impl: null, pUnk, CreateObjectFlags.TrackerObject, wrapperMaybe: null, out objMaybe))
return objMaybe!;

return GetObjectForIUnknownNative(pUnk);
}
Expand All @@ -439,9 +439,9 @@ public static object GetUniqueObjectForIUnknown(IntPtr unknown)
}

// Passing null as the ComWrapper implementation will use the globally registered wrappper (if available)
object? objMaybe = ComWrappers.GetOrCreateObjectForComInstanceInternal(impl: null, unknown, CreateObjectFlags.TrackerObject | CreateObjectFlags.UniqueInstance, wrapperMaybe: null);
if (objMaybe != null)
return objMaybe;
object? objMaybe;
if (ComWrappers.TryGetOrCreateObjectForComInstanceInternal(impl: null, unknown, CreateObjectFlags.TrackerObject | CreateObjectFlags.UniqueInstance, wrapperMaybe: null, out objMaybe))
return objMaybe!;

return GetUniqueObjectForIUnknownNative(unknown);
}
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/src/vm/ecalllist.h
Original file line number Diff line number Diff line change
Expand Up @@ -997,8 +997,8 @@ FCFuncEnd()
#ifdef FEATURE_COMWRAPPERS
FCFuncStart(gComWrappersFuncs)
QCFuncElement("GetIUnknownImplInternal", ComWrappersNative::GetIUnknownImpl)
QCFuncElement("GetOrCreateComInterfaceForObjectInternal", ComWrappersNative::GetOrCreateComInterfaceForObject)
QCFuncElement("GetOrCreateObjectForComInstanceInternal", ComWrappersNative::GetOrCreateObjectForComInstance)
QCFuncElement("TryGetOrCreateComInterfaceForObjectInternal", ComWrappersNative::TryGetOrCreateComInterfaceForObject)
QCFuncElement("TryGetOrCreateObjectForComInstanceInternal", ComWrappersNative::TryGetOrCreateObjectForComInstance)
FCFuncEnd()
#endif // FEATURE_COMWRAPPERS

Expand Down
63 changes: 41 additions & 22 deletions src/coreclr/src/vm/interoplibinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -478,16 +478,18 @@ namespace
CALL_MANAGED_METHOD_NORET(args);
}

void* GetOrCreateComInterfaceForObjectInternal(
bool TryGetOrCreateComInterfaceForObjectInternal(
_In_opt_ OBJECTREF impl,
_In_ OBJECTREF instance,
_In_ CreateComInterfaceFlags flags)
_In_ CreateComInterfaceFlags flags,
_Outptr_ void** wrapperRaw)
{
CONTRACT(void*)
CONTRACT(bool)
{
THROWS;
MODE_COOPERATIVE;
PRECONDITION(instance != NULL);
PRECONDITION(wrapperRaw != NULL);
}
CONTRACT_END;

Expand Down Expand Up @@ -586,20 +588,23 @@ namespace

GCPROTECT_END();

RETURN wrapperRawMaybe;
*wrapperRaw = wrapperRawMaybe;
RETURN (wrapperRawMaybe != NULL);
}

OBJECTREF GetOrCreateObjectForComInstanceInternal(
bool TryGetOrCreateObjectForComInstanceInternal(
_In_opt_ OBJECTREF impl,
_In_ IUnknown* identity,
_In_ CreateObjectFlags flags,
_In_opt_ OBJECTREF wrapperMaybe)
_In_opt_ OBJECTREF wrapperMaybe,
_Out_ OBJECTREF* objRef)
{
CONTRACT(OBJECTREF)
CONTRACT(bool)
{
THROWS;
MODE_COOPERATIVE;
PRECONDITION(identity != NULL);
PRECONDITION(objRef != NULL);
}
CONTRACT_END;

Expand Down Expand Up @@ -717,7 +722,8 @@ namespace

GCPROTECT_END();

RETURN gc.objRefMaybe;
*objRef = gc.objRefMaybe;
RETURN (gc.objRefMaybe != NULL);
}
}

Expand Down Expand Up @@ -952,20 +958,25 @@ namespace InteropLibImports
gc.wrapperMaybeRef = NULL; // No supplied wrapper here.

// Get wrapper for external object
gc.objRef = GetOrCreateObjectForComInstanceInternal(
bool success = TryGetOrCreateObjectForComInstanceInternal(
gc.implRef,
externalComObject,
externalObjectFlags,
gc.wrapperMaybeRef);
gc.wrapperMaybeRef,
&gc.objRef);

if (gc.objRef == NULL)
if (!success)
COMPlusThrow(kArgumentNullException);

// Get wrapper for managed object
*trackerTarget = GetOrCreateComInterfaceForObjectInternal(
success = TryGetOrCreateComInterfaceForObjectInternal(
gc.implRef,
gc.objRef,
trackerTargetFlags);
trackerTargetFlags,
trackerTarget);

if (!success)
COMPlusThrow(kArgumentException);

STRESS_LOG2(LF_INTEROP, LL_INFO100, "Created Target for External: 0x%p => 0x%p\n", OBJECTREFToObject(gc.objRef), *trackerTarget);
GCPROTECT_END();
Expand Down Expand Up @@ -1059,33 +1070,35 @@ namespace InteropLibImports

#ifdef FEATURE_COMWRAPPERS

void* QCALLTYPE ComWrappersNative::GetOrCreateComInterfaceForObject(
BOOL QCALLTYPE ComWrappersNative::TryGetOrCreateComInterfaceForObject(
_In_ QCall::ObjectHandleOnStack comWrappersImpl,
_In_ QCall::ObjectHandleOnStack instance,
_In_ INT32 flags)
_In_ INT32 flags,
_Outptr_ void** wrapper)
{
QCALL_CONTRACT;

void* wrapper = NULL;
bool success;

BEGIN_QCALL;

// Switch to Cooperative mode since object references
// are being manipulated.
{
GCX_COOP();
wrapper = GetOrCreateComInterfaceForObjectInternal(
success = TryGetOrCreateComInterfaceForObjectInternal(
ObjectToOBJECTREF(*comWrappersImpl.m_ppObject),
ObjectToOBJECTREF(*instance.m_ppObject),
(CreateComInterfaceFlags)flags);
(CreateComInterfaceFlags)flags,
wrapper);
}

END_QCALL;

return wrapper;
return success;
}

void QCALLTYPE ComWrappersNative::GetOrCreateObjectForComInstance(
BOOL QCALLTYPE ComWrappersNative::TryGetOrCreateObjectForComInstance(
_In_ QCall::ObjectHandleOnStack comWrappersImpl,
_In_ void* ext,
_In_ INT32 flags,
Expand All @@ -1096,6 +1109,8 @@ void QCALLTYPE ComWrappersNative::GetOrCreateObjectForComInstance(

_ASSERTE(ext != NULL);

bool success;

BEGIN_QCALL;

HRESULT hr;
Expand All @@ -1110,17 +1125,21 @@ void QCALLTYPE ComWrappersNative::GetOrCreateObjectForComInstance(
// are being manipulated.
{
GCX_COOP();
OBJECTREF newObj = GetOrCreateObjectForComInstanceInternal(
OBJECTREF newObj;
success = TryGetOrCreateObjectForComInstanceInternal(
ObjectToOBJECTREF(*comWrappersImpl.m_ppObject),
identity,
(CreateObjectFlags)flags,
ObjectToOBJECTREF(*wrapperMaybe.m_ppObject));
ObjectToOBJECTREF(*wrapperMaybe.m_ppObject),
&newObj);

// Set the return value
retValue.Set(newObj);
}

END_QCALL;

return success;
}

void QCALLTYPE ComWrappersNative::GetIUnknownImpl(
Expand Down
7 changes: 4 additions & 3 deletions src/coreclr/src/vm/interoplibinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ class ComWrappersNative
_Out_ void** fpAddRef,
_Out_ void** fpRelease);

static void* QCALLTYPE GetOrCreateComInterfaceForObject(
static BOOL QCALLTYPE TryGetOrCreateComInterfaceForObject(
_In_ QCall::ObjectHandleOnStack comWrappersImpl,
_In_ QCall::ObjectHandleOnStack instance,
_In_ INT32 flags);
_In_ INT32 flags,
_Outptr_ void** wrapperRaw);

static void QCALLTYPE GetOrCreateObjectForComInstance(
static BOOL QCALLTYPE TryGetOrCreateObjectForComInstance(
_In_ QCall::ObjectHandleOnStack comWrappersImpl,
_In_ void* externalComObject,
_In_ INT32 flags,
Expand Down

0 comments on commit dad304e

Please sign in to comment.