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

STGMEDIUM is invalid under the new interface generation #197

Closed
weltkante opened this issue Mar 16, 2021 · 9 comments
Closed

STGMEDIUM is invalid under the new interface generation #197

weltkante opened this issue Mar 16, 2021 · 9 comments

Comments

@weltkante
Copy link

My usual test for COM interop is to implement IDataObject, put it on the clipboard, and retrieve some data. I'm getting nowhere with this, STGMEDIUM is incompatible with the classic marshaler and the runtime will fail interop calls.

incomplete example code
static class Program
{
    [STAThread]
    static void Main()
    {
        Application.OleRequired();
        PInvoke.OleSetClipboard(new CustomDataObject());
        Clipboard.GetData("Foo");
    }
}

class CustomDataObject : IDataObject
{
    public const int E_FAIL = -2147467259;

    public unsafe void GetData(FORMATETC* pformatetcIn, out STGMEDIUM pmedium)
    {
        throw new NotImplementedException();
    }

    public unsafe void GetDataHere(FORMATETC* pformatetc, ref STGMEDIUM pmedium)
    {
        throw new NotImplementedException();
    }

    public unsafe void QueryGetData(FORMATETC* pformatetc)
    {
        throw new NotImplementedException();
    }

    public unsafe void GetCanonicalFormatEtc(FORMATETC* pformatectIn, FORMATETC* pformatetcOut)
    {
        throw new NotImplementedException();
    }

    public unsafe void SetData(FORMATETC* pformatetc, in STGMEDIUM pmedium, BOOL fRelease)
    {
        throw new NotImplementedException();
    }

    public unsafe void EnumFormatEtc(uint dwDirection, out IEnumFORMATETC ppenumFormatEtc)
    {
        if ((DATADIR)dwDirection == DATADIR.DATADIR_GET)
        {
            FORMATETC fmt = default;
            fmt.cfFormat = (ushort)DataFormats.GetFormat("Foo").Id;
            fmt.dwAspect = (uint)DVASPECT.DVASPECT_CONTENT;
            fmt.lindex = -1;
            fmt.ptd = null;
            fmt.tymed = (uint)TYMED.TYMED_HGLOBAL;

            if (PInvoke.SHCreateStdEnumFmtEtc(1, &fmt, out var enumerator).Succeeded)
            {
                ppenumFormatEtc = enumerator;
                return;
            }
        }

        throw Marshal.GetExceptionForHR(E_FAIL);
    }

    public unsafe void DAdvise(FORMATETC* pformatetc, uint advf, IAdviseSink pAdvSink, uint* pdwConnection)
    {
        throw new NotImplementedException();
    }

    public void DUnadvise(uint dwConnection)
    {
        throw new NotImplementedException();
    }

    public void EnumDAdvise(out IEnumSTATDATA ppenumAdvise)
    {
        throw new NotImplementedException();
    }
}

Since this failed before I could write a complete test scenario my test code does not implement IDataObject sufficiently enough to actually retrieve data, .NET will fail before entering either GetData or GetDataHere. You can see one of the exceptions if you enable first chance exception and run above code:

System.TypeLoadException: 'Could not load type '_Anonymous_e__Union' from assembly 'WinFormsApp1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' because it contains an object field at offset 0 that is incorrectly aligned or overlapped by a non-object field.'

@AArnott
Copy link
Member

AArnott commented Mar 16, 2021

Given the title of your issue, did this work with an older version?

@weltkante
Copy link
Author

weltkante commented Mar 16, 2021

Well, when using structs you weren't calling into the classic marshaler, so the STGMEDIUM was valid, just containing pointers to structs. Obviously with the old struct-based approach you couldn't implement IDataObject but you could call IDataObject and provide your own STGMEDIUM. I didn't test it, but it should have worked, considering there was no marshaling going on, just pointers to structs.

The problem is that you are now putting a marshaled .NET object into a union, which is something the classic marshaler doesn't allow (since it doesn't know when the union field is a reference to a .NET object and when its a plain value)

Other unions like VARIANT probably have the same problem now. There might be some more unions with COM interfaces in them.

@AArnott
Copy link
Member

AArnott commented Mar 16, 2021

BTW you can turn off interface generation using the NativeMethods.json file to get the old behavior back.

@weltkante
Copy link
Author

Yeah I noticed that but couldn't figure out what the option is. My Visual Studio is also terribly unhappy about something in the generator and throws several "gold bar" errors per minute once I start using the generator, so if the json schema is supposed to provide intellisense VS is probably too broken to deliver it at the moment.

Anyways, as far as unions go, its not easy to fix. C is a language which only has dumb unions and the context for the union is provided outside in a containing struct or as a separate parameter (if the union is a function call parameter). This means the projection can either also be dumb, not projecting COM interfaces inside unions, using pointers instead and leave union handling and unpacking COM references to the caller; or it needs additional information to properly project unions, namely the context to disambiguate the union.

When doing handwritten projections I usually lift the union API into its containing struct/class where the disambuation field is available, this typically matches the API just fine since often these are nested unions anyways which aren't supposed to be used as standalone types in the first place.

It would probably be worth to do a study over the metadata package to see if all unions in the package have disambiguation information available in their direct usage site, and if not, classify the cases where the disambiguation data comes from somewhere else.

If its always available in the direct usage site it would be worth adding metadata to the metadata package to describe how the container is disambiguating unions, allowing projections to generate higher quality APIs for unions.

@AArnott
Copy link
Member

AArnott commented Mar 16, 2021

if the json schema is supposed to provide intellisense VS is probably too broken to deliver it at the moment.

It is, if you have the $schema property defined as explained in the readme.

There is a top-level property called allowMarshaling which you can set to false to go back to the old mode.

@prajaybasu
Copy link

Is there any update on this issue? I would like to be able to pass nulls at least.

@AArnott
Copy link
Member

AArnott commented Apr 28, 2021

I haven't had a chance yet to circle back and improve the COM interface generation. The workaround I prescribe above still applies though.

@prajaybasu
Copy link

The workaround I prescribe above still applies though.

I went with a C++/WinRT app instead. As a beginner, it was easier to switch to C++ than turning off interface generation.

Maybe I'll come back to C# when I know a bit more about COM.

@AArnott
Copy link
Member

AArnott commented Jun 1, 2022

Duplicate of #292

@AArnott AArnott marked this as a duplicate of #292 Jun 1, 2022
@AArnott AArnott closed this as completed Jun 1, 2022
AArnott added a commit that referenced this issue May 1, 2023
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

No branches or pull requests

3 participants