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

Regression in .Net 5 Preview 7 - MarshalDirectiveException is thrown when using the UnmanagedType.HString #39827

Closed
DmitryGaravsky opened this issue Jul 23, 2020 · 6 comments
Labels
area-Interop-coreclr question Answer questions and provide assistance, not an issue with source code or documentation.
Milestone

Comments

@DmitryGaravsky
Copy link
Contributor

DmitryGaravsky commented Jul 23, 2020

Description

For now, some COM interop code fails with the MarshalDirectiveException:

System.Runtime.InteropServices.MarshalDirectiveException: 'Cannot marshal 'parameter #1': Invalid managed/unmanaged type combination.'

Our unit tests detected this issue immediately. (we maintain unit tests for all the released and preview environments like .net core 5 in our codebase).
Thus it is an important breaking change for us.

Configuration

OS Version: Microsoft Windows [Version 10.0.18362.535]
.Net Framework Version: Microsoft (R) .NET CLR Version Tool Version 4.8.3928.0
.Net Core Version: SDK Version: 5.0.100-preview.7.20366.6

Regression?

Yes, this works correctly until .Net Core 5 Preview 7

Other information

Here is the code snippet which fail under .Net Core 5 Preview 7:

class Program {
    static void Main(string[] args) {
        object iface;
        int res = RoGetActivationFactory(
                    "Windows.UI.Notifications.ToastNotificationManager",
                    new Guid("50AC103F-D235-4598-BBEF-98FE4D1A3AD4"),
                    out iface
                );
    }
    [SecuritySafeCritical, DllImport("Combase.dll")]
    static extern int RoGetActivationFactory(
            [MarshalAs(UnmanagedType.HString)] string classId,
            [In, MarshalAs(UnmanagedType.LPStruct)] Guid guid,
            [Out, MarshalAs(UnmanagedType.IUnknown)] out object iface);
}

RoGetActivationFactory function:

HRESULT RoGetActivationFactory(
  HSTRING activatableClassId,
  REFIID  iid,
  void    **factory
);
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Interop-coreclr untriaged New issue has not been triaged by the area owner labels Jul 23, 2020
@AaronRobinsonMSFT
Copy link
Member

@DmitryGaravsky The built-in support for WinRT API has been removed. This support was removed as a documented breaking change - see #37672. The removal of built-in WinRT support does seem to make the marshalling of HString to be less interesting, but we welcome feedback if this is a requirement even in cases where WinRT isn't being used. The replacement for built-in WinRT is C#/WinRT which is a much more stable and servicable solution going forward. We have worked closely with that team to help them on the path to delivering this replacement.

Please let us know if the support for UnmanagedType.HString is a hard requirement for your scenario even without the built-in WinRT support.

@AaronRobinsonMSFT
Copy link
Member

/cc @jkoritzinsky

@DmitryGaravsky
Copy link
Contributor Author

Hi Aaron, thanks for participating

Could you please describe in detail why "removing built-in support for WinRT API" affects the PInvoke to Combase.dll in this way?
DllImport("Combase.dll")
And why the exception says just about the marshaling error?

I understand that under the hood the RoGetActivationFactory function is exported by the api-ms-win-core-winrt-l1-1-0.dll. This library is still included in Windows, so it is not quite clear for us why PInvoke fails for now. Are there some ways to tune this behavior?
At this moment I have found the following WA which works to me at first glance.
Can you validate my thought?

object iface;
int res = RoGetActivationFactory(
        HSTRING.FromString("Windows.UI.Notifications.ToastNotificationManager"),
        new Guid("50AC103F-D235-4598-BBEF-98FE4D1A3AD4"),
        out iface
    );
//...
[SecuritySafeCritical, DllImport("api-ms-win-core-winrt-l1-1-0.dll")]
static extern int RoGetActivationFactory(
    HSTRING classId,
    [In, MarshalAs(UnmanagedType.LPStruct)] Guid guid,
    [Out, MarshalAs(UnmanagedType.IUnknown)] out object iface);

[StructLayout(LayoutKind.Sequential), SecuritySafeCritical]
struct HSTRING {
    readonly IntPtr handle;
    public static HSTRING FromString(string s) {
        IntPtr h = Marshal.AllocHGlobal(IntPtr.Size);
        Marshal.ThrowExceptionForHR(WindowsCreateString(s, s.Length, h));
        return Marshal.PtrToStructure<HSTRING>(h);
    }
    //
    [DllImport("api-ms-win-core-winrt-string-l1-1-0.dll", CallingConvention = CallingConvention.StdCall)]
    static extern int WindowsCreateString(
        [MarshalAs(UnmanagedType.LPWStr)] string sourceString, int length, [Out] IntPtr hstring);
}

Regarding our scenario - we heavily use the demonstrated API to implement a UI component for showing toast notifications from desktop applications( WinForms/WPF).

@jkoritzinsky
Copy link
Member

The P/Invoke fails because of the usage of [MarshalAs(UnmanagedType.HString)]. The HSTRING type was introduced with WinRT in Windows 8 and continues to only be used with WinRT-related APIs (including RoGetActivationFactory).

Your workaround has a memory leak: It never releases the created HSTRING.

You should add a wrapper for WindowsDeleteString as you have for WindowsCreateString and change your usage from above to something more similar to the following:

object iface;
IntPtr hstring = HSTRING.FromString("Windows.UI.Notifications.ToastNotificationManager");
try
{
    int res = RoGetActivationFactory(
            hstring,
            new Guid("50AC103F-D235-4598-BBEF-98FE4D1A3AD4"),
            out iface
        );
}
finally
{
       HSTRING.DeleteString(hstring); // A wrapper for WindowsDeleteString
}

However, even with this workaround, you'll get an iface instance that uses the built-in marshaling system and that doesn't support WinRT interfaces from winmd files or ComImport interfaces marked with [InterfaceType(ComInterfaceType.InterfaceIsIInspectable)]. If you manually import all of the interfaces you use with [ComImport] and [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] and add dummy slots for the IInspectable members, you could get this to work. However, it is much simpler to use C#/WinRT as Aaron mentioned.

@AaronRobinsonMSFT
Copy link
Member

@DmitryGaravsky These are all good questions.

Could you please describe in detail why "removing built-in support for WinRT API" affects the PInvoke to Combase.dll in this way?

It impacts this API because the UnmanagedType.HString marshalling type was a part of the WinRT system. The HSTRING type was created for WinRT and as such we removed it because if WinRT types are not longer built-in then there is no reason to provide that support. The new support for WinRT is handled by C#/WinRT and that tooling does have support for HSTRING, but like the old style does permit usage of string and automatically converts during any calls.

And why the exception says just about the marshaling error?

Because the function call itself isn't the problem, but rather the marshalling of string to HSTRING.

I understand that under the hood the RoGetActivationFactory function is exported by the api-ms-win-core-winrt-l1-1-0.dll. This library is still included in Windows, so it is not quite clear for us why PInvoke fails for now. Are there some ways to tune this behavior?

The marshalling is the issue here, not the call itself. The API might actually fail though because the runtime is no longer marshalling the string to an HSTRING.

At this moment I have found the following WA which works to me at first glance.
Can you validate my thought?

That code will work as expected as it relates to the call to RoGetActivationFactory. However, the resulting object, iface, will not be what is desired. The type ToastNotificationManager is no longer provided "built-in" to the .NET runtime since C#/WinRT is now handling all those uses. I would recommend looking at C#/WinRT or asking how to consume this type on the cswinrt github.

@AaronRobinsonMSFT AaronRobinsonMSFT added question Answer questions and provide assistance, not an issue with source code or documentation. and removed untriaged New issue has not been triaged by the area owner labels Jul 23, 2020
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 5.0.0 milestone Jul 23, 2020
@AaronRobinsonMSFT
Copy link
Member

@DmitryGaravsky I am going to close this for now as the described behavior is expected. Feel free to continue the conversation here or tag myself or @jkoritzinsky if you report issues on the CsWinRT repo. Thanks.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Interop-coreclr question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

4 participants