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.

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

Emit CCW entrypoint functions and populate vtbl #825

Merged
merged 1 commit into from
Dec 9, 2022
Merged

Emit CCW entrypoint functions and populate vtbl #825

merged 1 commit into from
Dec 9, 2022

Conversation

AArnott
Copy link
Member

@AArnott AArnott commented Dec 9, 2022

The new elements are the [UnmanagedCallersOnly] functions, the PopulateVTable method, and the ComHelpers class.

The PopulateVTable method skips initializing IUnknown and IDispatch inherited members because we have nothing to forward them to. We count on ComWrappers or the app to supply these.

Samples

Here is the ComHelpers class:

internal static unsafe partial class ComHelpers
{
    private static readonly winmdroot.Foundation.HRESULT COR_E_OBJECTDISPOSED = (winmdroot.Foundation.HRESULT)unchecked((int)0x80131622);
    private static readonly winmdroot.Foundation.HRESULT S_OK = (winmdroot.Foundation.HRESULT)0;

    internal static winmdroot.Foundation.HRESULT UnwrapCCW<This, TInterface>(TThis* @this, out TInterface @object)
        where TThis : unmanaged
        where TInterface : class
    {
        @object = ComWrappers.ComInterfaceDispatch.GetInstance<TInterface>((ComWrappers.ComInterfaceDispatch*)@this);
        return @object is null ? COR_E_OBJECTDISPOSED : S_OK;
    }
}

Here is a relatively small and straightforward COM interface with just a couple unique methods:

[Guid("00020403-0000-0000-C000-000000000046")]
[global::System.CodeDom.Compiler.GeneratedCode("Microsoft.Windows.CsWin32", "0.2.162-beta+74a0fe322c")]
internal unsafe partial struct ITypeComp
{
	/// <inheritdoc cref="QueryInterface(global::System.Guid*, void**)"/>
	internal unsafe winmdroot.Foundation.HRESULT QueryInterface(in global::System.Guid riid, out void* ppvObject)
	{
		fixed (void** ppvObjectLocal = &ppvObject)
		{
			fixed (global::System.Guid* riidLocal = &riid)
			{
				winmdroot.Foundation.HRESULT __result = this.QueryInterface(riidLocal, ppvObjectLocal);
				return __result;
			}
		}
	}

	public unsafe winmdroot.Foundation.HRESULT QueryInterface(global::System.Guid* riid, void** ppvObject)
	{
		fixed (ITypeComp* pThis = &this)
			return ((delegate *unmanaged [Stdcall]<ITypeComp*,global::System.Guid* ,void** ,winmdroot.Foundation.HRESULT>)lpVtbl[0])(pThis, riid, ppvObject);
	}

	public uint AddRef()
	{
		fixed (ITypeComp* pThis = &this)
			return ((delegate *unmanaged [Stdcall]<ITypeComp*,uint>)lpVtbl[1])(pThis);
	}

	public uint Release()
	{
		fixed (ITypeComp* pThis = &this)
			return ((delegate *unmanaged [Stdcall]<ITypeComp*,uint>)lpVtbl[2])(pThis);
	}

	/// <inheritdoc cref="Bind(winmdroot.Foundation.PWSTR, uint, ushort, winmdroot.System.Com.ITypeInfo**, winmdroot.System.Com.DESCKIND*, winmdroot.System.Com.BINDPTR*)"/>
	internal unsafe void Bind(winmdroot.Foundation.PWSTR szName, uint lHashVal, ushort wFlags, winmdroot.System.Com.ITypeInfo** ppTInfo, out winmdroot.System.Com.DESCKIND pDescKind, out winmdroot.System.Com.BINDPTR pBindPtr)
	{
		fixed (winmdroot.System.Com.BINDPTR* pBindPtrLocal = &pBindPtr)
		{
			fixed (winmdroot.System.Com.DESCKIND* pDescKindLocal = &pDescKind)
			{
				this.Bind(szName, lHashVal, wFlags, ppTInfo, pDescKindLocal, pBindPtrLocal);
			}
		}
	}

	[UnmanagedCallersOnly(CallConvs = new []{typeof(CallConvStdcall)})]
	private static winmdroot.Foundation.HRESULT Bind(ITypeComp* pThis, winmdroot.Foundation.PWSTR szName, uint lHashVal, ushort wFlags, winmdroot.System.Com.ITypeInfo** ppTInfo, winmdroot.System.Com.DESCKIND* pDescKind, winmdroot.System.Com.BINDPTR* pBindPtr)
	{
		try
		{
			winmdroot.Foundation.HRESULT hr = ComHelpers.UnwrapCCW(pThis, out Interface @object);
			if (hr.Failed)
			{
				return hr;
			}
			return @object.Bind(szName, lHashVal, wFlags, ppTInfo, pDescKind, pBindPtr);
		}
		catch (Exception ex)
		{
			return (winmdroot.Foundation.HRESULT)ex.HResult;
		}
	}

	public unsafe void Bind(winmdroot.Foundation.PWSTR szName, uint lHashVal, ushort wFlags, winmdroot.System.Com.ITypeInfo** ppTInfo, winmdroot.System.Com.DESCKIND* pDescKind, winmdroot.System.Com.BINDPTR* pBindPtr)
	{
		fixed (ITypeComp* pThis = &this)
			((delegate *unmanaged [Stdcall]<ITypeComp*,winmdroot.Foundation.PWSTR ,uint ,ushort ,winmdroot.System.Com.ITypeInfo** ,winmdroot.System.Com.DESCKIND* ,winmdroot.System.Com.BINDPTR* ,winmdroot.Foundation.HRESULT>)lpVtbl[3])(pThis, szName, lHashVal, wFlags, ppTInfo, pDescKind, pBindPtr).ThrowOnFailure();
	}

	[UnmanagedCallersOnly(CallConvs = new []{typeof(CallConvStdcall)})]
	private static winmdroot.Foundation.HRESULT BindType(ITypeComp* pThis, winmdroot.Foundation.PWSTR szName, uint lHashVal, winmdroot.System.Com.ITypeInfo** ppTInfo, winmdroot.System.Com.ITypeComp** ppTComp)
	{
		try
		{
			winmdroot.Foundation.HRESULT hr = ComHelpers.UnwrapCCW(pThis, out Interface @object);
			if (hr.Failed)
			{
				return hr;
			}
			return @object.BindType(szName, lHashVal, ppTInfo, ppTComp);
		}
		catch (Exception ex)
		{
			return (winmdroot.Foundation.HRESULT)ex.HResult;
		}
	}

	public unsafe void BindType(winmdroot.Foundation.PWSTR szName, uint lHashVal, winmdroot.System.Com.ITypeInfo** ppTInfo, winmdroot.System.Com.ITypeComp** ppTComp)
	{
		fixed (ITypeComp* pThis = &this)
			((delegate *unmanaged [Stdcall]<ITypeComp*,winmdroot.Foundation.PWSTR ,uint ,winmdroot.System.Com.ITypeInfo** ,winmdroot.System.Com.ITypeComp** ,winmdroot.Foundation.HRESULT>)lpVtbl[4])(pThis, szName, lHashVal, ppTInfo, ppTComp).ThrowOnFailure();
	}

	internal struct Vtbl
	{
		internal delegate *unmanaged [Stdcall]<ITypeComp*,global::System.Guid* ,void** ,winmdroot.Foundation.HRESULT> QueryInterface_1;
		internal delegate *unmanaged [Stdcall]<ITypeComp*,uint> AddRef_2;
		internal delegate *unmanaged [Stdcall]<ITypeComp*,uint> Release_3;
		internal delegate *unmanaged [Stdcall]<ITypeComp*,winmdroot.Foundation.PWSTR ,uint ,ushort ,winmdroot.System.Com.ITypeInfo** ,winmdroot.System.Com.DESCKIND* ,winmdroot.System.Com.BINDPTR* ,winmdroot.Foundation.HRESULT> Bind_4;
		internal delegate *unmanaged [Stdcall]<ITypeComp*,winmdroot.Foundation.PWSTR ,uint ,winmdroot.System.Com.ITypeInfo** ,winmdroot.System.Com.ITypeComp** ,winmdroot.Foundation.HRESULT> BindType_5;
	} 
	internal static void PopulateVTable(Vtbl* vtable)
	{
		vtable->Bind_4 = &Bind;
		vtable->BindType_5 = &BindType;
	}
	private void** lpVtbl;
	/// <summary>The IID guid for this interface.</summary>
	/// <value>{00020403-0000-0000-c000-000000000046}</value>
	internal static readonly Guid IID_Guid = new Guid(0x00020403, 0x0000, 0x0000, 0xC0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46);
	[Guid("00020403-0000-0000-C000-000000000046"),InterfaceType(ComInterfaceType.InterfaceIsIUnknown),ComImport()]
	internal interface Interface
	{
		[PreserveSig()]
		unsafe winmdroot.Foundation.HRESULT Bind(winmdroot.Foundation.PWSTR szName, uint lHashVal, ushort wFlags, winmdroot.System.Com.ITypeInfo** ppTInfo, winmdroot.System.Com.DESCKIND* pDescKind, winmdroot.System.Com.BINDPTR* pBindPtr);

		[PreserveSig()]
		unsafe winmdroot.Foundation.HRESULT BindType(winmdroot.Foundation.PWSTR szName, uint lHashVal, winmdroot.System.Com.ITypeInfo** ppTInfo, winmdroot.System.Com.ITypeComp** ppTComp);
	}
}

The ITypeInfo interface is interesting because it defines methods that do not return HRESULT. In such cases, we throw and/or allow managed exceptions to propagate to the native caller:

[UnmanagedCallersOnly(CallConvs = new []{typeof(CallConvStdcall)})]
private static void ReleaseTypeAttr(ITypeInfo* pThis, winmdroot.System.Com.TYPEATTR* pTypeAttr)
{
    winmdroot.Foundation.HRESULT hr = ComHelpers.UnwrapCCW(pThis, out Interface @object);
    hr.ThrowOnFailure();
    @object.ReleaseTypeAttr(pTypeAttr);
}

In another case, the emitted interface includes properties declared as properties, so we emit special code to handle that case too:

[UnmanagedCallersOnly(CallConvs = new []{typeof(CallConvStdcall)})]
private static winmdroot.Foundation.HRESULT get_Data(IInkExtendedProperty* pThis, winmdroot.System.Com.VARIANT* Data)
{
    try
    {
        winmdroot.Foundation.HRESULT hr = ComHelpers.UnwrapCCW(pThis, out Interface @object);
        if (hr.Failed)
        {
            return hr;
        }
        *Data = @object.Data;
        return winmdroot.Foundation.HRESULT.S_OK;
    }
    catch (Exception ex)
    {
        return (winmdroot.Foundation.HRESULT)ex.HResult;
    }
}

[UnmanagedCallersOnly(CallConvs = new []{typeof(CallConvStdcall)})]
private static winmdroot.Foundation.HRESULT put_Data(IInkExtendedProperty* pThis, winmdroot.System.Com.VARIANT Data)
{
    try
    {
        winmdroot.Foundation.HRESULT hr = ComHelpers.UnwrapCCW(pThis, out Interface @object);
        if (hr.Failed)
        {
            return hr;
        }
        @object.Data = Data;
        return winmdroot.Foundation.HRESULT.S_OK;
    }
    catch (Exception ex)
    {
        return (winmdroot.Foundation.HRESULT)ex.HResult;
    }
}

Closes #751

@AArnott
Copy link
Member Author

AArnott commented Dec 9, 2022

@tannergooding one question I and @JeremyKuhne have for you particularly is whether I took the right approach for the CCW on methods that do not return HRESULT with regard to reporting errors to the caller. Is throwing managed exceptions back to them acceptable? If not, what is the alternative?

@AArnott AArnott merged commit 187018b into main Dec 9, 2022
@AArnott AArnott deleted the fix751 branch December 9, 2022 05:07
@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Dec 9, 2022

Is throwing managed exceptions back to them acceptable? If not, what is the alternative?

On Windows it is supported via SEH. I'm not a fan of evangelizing that support because it won't be present on other platforms and writing .NET code that is platform specific is generally a code smell for me. I get the "this is only for Windows" but that, to me, is a cheap trick rather than just doing what is correct for COM and that is fail fast. COM has long held exceptions don't propagate across binary boundaries and .NET has done a poor job of enforcing that behavior.

The TL;DR here is yes this will work but it is setting expectations that likely won't be met if we ever port to another platform.

@AArnott
Copy link
Member Author

AArnott commented Dec 10, 2022

Thanks, @AaronRobinsonMSFT.
So you say the alternative is fail fast. So... you'd rather see the process exit immediately when an exception is thrown from managed code in a COM method that doesn't return HRESULT? I just want to clarify, because we do have an opportunity to have that behavior here. Although CsWin32 is currently scoped to Win32 and related metadata, so a Windows-only behavior isn't a bad thing. Although CsWin32 could be useful on other platforms eventually.... it's just not a priority or even a customer ask right now.

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Dec 10, 2022

Confirming here so we are on the same page, the implementation of the COM interface is in managed code, right? We are talking about CCWs.

So... you'd rather see the process exit immediately when an exception is thrown from managed code in a COM method that doesn't return HRESULT?

Yes, I would prefer the fail fast on the managed side to throwing a managed exception and having it propagate. Coming back into unmanaged (i.e., native) code means we are assuming the caller is going to have set up a SEH handler. In my experience this is exceedingly rare and results in DMPs that aren't always that easy to root cause and if it was detected I'm not sure what the caller expects to do with that exception. It also assumes the transition from managed to SEH is something the caller thought about - also unlikely.

COM interfaces that don't return an HRESULT indicate they don't expect a recoverable error in any form, that is how I've always interpreted them. The ITypeInfo members that return void, such as ReleaseVarDesc(), indicate to me that the callee isn't expecting or designed to handle invalid input. One could debate whether NULL is valid or not, but it is taking an arbitrary pointer to release and giving a invalid pointer is expensive to detect and unlikely to have a lot of utility, so fail fast. Alternatively the callee could defer to the dependent memory allocator, which is typically "fail fast" also.

If we are looking to support COM, actual COM not any of the various "subsets", then a member either returns an HRESULT, doesn't expect to have invalid input or swallows all failures and nicely returns. Letting exceptions propagate out from a COM member isn't supported and isn't something the .NET Interop team is going to recommend going forward.

/cc @jkoritzinsky @elinor-fung

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to generate vtable callback generation
2 participants