From dad304e6c50544f9bd23f6b1173781f82559c4fe Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 12 Mar 2020 14:15:36 -0700 Subject: [PATCH] Switch to Try semantics --- .../Runtime/InteropServices/ComWrappers.cs | 38 +++++------ .../InteropServices/Marshal.CoreCLR.cs | 16 ++--- src/coreclr/src/vm/ecalllist.h | 4 +- src/coreclr/src/vm/interoplibinterface.cpp | 63 ++++++++++++------- src/coreclr/src/vm/interoplibinterface.h | 7 ++- 5 files changed, 74 insertions(+), 54 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs index cd4780d970d0c..731437a123009 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs @@ -119,8 +119,8 @@ private struct ComInterfaceInstance /// The generated COM interface that can be passed outside the .NET runtime. 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; @@ -132,20 +132,21 @@ public IntPtr GetOrCreateComInterfaceForObject(object instance, CreateComInterfa /// The implemenentation to use when creating the COM representation. /// The managed object to expose outside the .NET runtime. /// Flags used to configure the generated interface. - /// The generated COM interface that can be passed outside the .NET runtime or IntPtr.Zero if it could not be created. + /// The generated COM interface that can be passed outside the .NET runtime or IntPtr.Zero if it could not be created. + /// Returns true if a CEM representation could be created, false otherwise /// /// If is null, the global instance (if registered) will be used. /// - 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); /// /// Compute the desired Vtable for respecting the values of . @@ -184,11 +185,11 @@ internal static IntPtr GetOrCreateComInterfaceForObjectInternal(ComWrappers? imp /// Returns a managed object associated with the supplied external COM object. 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!; } /// @@ -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!; } /// @@ -241,24 +242,23 @@ public object GetOrRegisterObjectForComInstance(IntPtr externalComObject, Create /// Object to import for usage into the .NET runtime. /// Flags used to describe the external object. /// The to be used as the wrapper for the external object. - /// Returns a managed object associated with the supplied external COM object or null if it could not be created. + /// The managed object associated with the supplied external COM object or null if it could not be created. + /// Returns true if a managed object could be retrieved/created, false otherwise /// /// If is null, the global instance (if registered) will be used. /// - 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); /// /// Called when a request is made for a collection of objects to be released outside of normal object or COM interface lifetime. diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs index 54aa45f6bbee3..11b0fdf1241cb 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs @@ -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); @@ -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); } @@ -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); } diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h index 4025d2162fd11..03b2d7c3eee32 100644 --- a/src/coreclr/src/vm/ecalllist.h +++ b/src/coreclr/src/vm/ecalllist.h @@ -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 diff --git a/src/coreclr/src/vm/interoplibinterface.cpp b/src/coreclr/src/vm/interoplibinterface.cpp index 1fba4f62a87fd..ea2035b9bd930 100644 --- a/src/coreclr/src/vm/interoplibinterface.cpp +++ b/src/coreclr/src/vm/interoplibinterface.cpp @@ -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; @@ -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; @@ -717,7 +722,8 @@ namespace GCPROTECT_END(); - RETURN gc.objRefMaybe; + *objRef = gc.objRefMaybe; + RETURN (gc.objRefMaybe != NULL); } } @@ -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(); @@ -1059,14 +1070,15 @@ 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; @@ -1074,18 +1086,19 @@ void* QCALLTYPE ComWrappersNative::GetOrCreateComInterfaceForObject( // 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, @@ -1096,6 +1109,8 @@ void QCALLTYPE ComWrappersNative::GetOrCreateObjectForComInstance( _ASSERTE(ext != NULL); + bool success; + BEGIN_QCALL; HRESULT hr; @@ -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( diff --git a/src/coreclr/src/vm/interoplibinterface.h b/src/coreclr/src/vm/interoplibinterface.h index 2ed54cfc90706..5ea4b55c28f50 100644 --- a/src/coreclr/src/vm/interoplibinterface.h +++ b/src/coreclr/src/vm/interoplibinterface.h @@ -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,