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

Dynamic keyword not working against COM objects #12587

Closed
RickStrahl opened this issue Apr 25, 2019 · 23 comments · Fixed by #33060
Closed

Dynamic keyword not working against COM objects #12587

RickStrahl opened this issue Apr 25, 2019 · 23 comments · Fixed by #33060

Comments

@RickStrahl
Copy link

RickStrahl commented Apr 25, 2019

I'm working on porting one of my applications (Markdown Monster) to .NET Core 3.0 and it's going good except for my COM interop functionality that uses dynamic to access various COM components inside of the Web Browser control.

Using raw Reflection instead of dynamic works, but it would be a lot of code in my case that has to be converted to make the Interop with with the more clumsy syntax and type casting.

Specifically I'm getting back objects from the Web Browser control and then use dynamic to call other functionality. All of that doesn't work with .NET Core. I previously mentioned this and at the time it looked like there was already work underway to make this work in 3.0, with no fixes scheduled for 2.x. I talked about this in a blog post here. The original discussion I referenced that mentioned fixes for 3.0 where in https://github.com/dotnet/corefx/issues/32630.

But now we're in 3.0 Preview and it's still not working.

Here's what I am doing:

// Get the JavaScript Ace Editor Instance
dynamic doc = WebBrowser.Document;

// fails
dynamic window = doc.parentWindow;

image

Replacing the code with manual reflection does work:

// this does
object window = ReflectionUtils.GetPropertyCom(doc, "parentWindow");

Status

There were previous issues open on this some time ago and at the time the word was that .NET Core 3.0 was going to fix this. It's not working on 2.x either at the time the call was won't fix for 2.x but will fix for 3.0.

Apparently it's not fixed in 3.0.

Is there any definite word on whether this will get addressed?

For me this is pretty big blocker in this app. I have tons of interop calls, and using dynamic is a key language feature that makes this code much more manageable (and also more performant due dynamic's internal caching etc) than using Reflection.

FWIW I already have an abstraction layer around the Editor/COM interop calls, but dynamic is such a core feature that it never occurred to me to abstract that further. I think if you want decent Windows Desktop support dynamic really should be working with COM objects.

Using latest .NET Core 3.0 Preview 4 SDK.

@jkotas
Copy link
Member

jkotas commented Apr 25, 2019

cc @AaronRobinsonMSFT

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Apr 25, 2019

@jkotas Any chance we can move this issue to the coreclr repo?

cc @jeffschwMSFT

@jkotas jkotas transferred this issue from dotnet/corefx Apr 25, 2019
@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Apr 25, 2019

@RickStrahl Given the current priorities and the possible workarounds this isn't support we are planning on adding in 3.0. However, this is something we will consider in a future release.

@RickStrahl
Copy link
Author

RickStrahl commented Apr 25, 2019

That's disappointing... but I get it. This encompasses a large set of functionality, but it's not like that code doesn't exist already in full framework. For COM this should probably even live in Windows.Compatibility most likely.

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Apr 25, 2019

@RickStrahl Thanks for understanding.

This encompasses a large set of functionality, but it's not like that code doesn't exist already in full framework.

Indeed. I can assure you this wasn't an easy decision. In the spirit of transparency let me elaborate on the calculus used to make this decision.

The above assumption is correct and in many cases that fact is enough to let us port code over. However, in the case of dynamic supporting COM the amount of code required to add was on the order of thousands of lines of customized special cases. In general this is not much of an issue but adding this much code to the Dynamic Language Runtime (DLR) for .NET Core would have also required pulling in a substantial amount of testing as well. The combined cost of product work with testing debt was too much to pay in this release. There is also the unfortunate fact that many of the people with expert knowledge about the DLR are no longer on the .NET team which makes these kind of large feature changes difficult to perform in confidence. The final reason compounds all of the above and that is customer need. To be sure there are many COM users of the dynamic keyword but compared to other interop scenarios the desire for this one is smaller and even though the workaround is incredibly annoying and ugly - a workaround does exist.

There is no question this support can be added back in the future, but for right now it is just too expensive.

@govert
Copy link
Contributor

govert commented Apr 26, 2019

@AaronRobinsonMSFT

This means the C# 4.0 COM interop work that implicitly uses 'dynamic' for IDispatch interfaces will not yet be available under .NET Core 3.0.

A consequence would be that most Office automation code cannot move from .NET Framework to .NET Core 3.0. This is because, even where Primary Interop Assemblies are used for early-bound COM access, the implicit interpretation of IDispatch as "dynamic" under C# is used extensively. For example, this code driving Excel works fine under C# 4 / NET Framework 4, but will not compile under .NET Core 3.0, even though everything seems to be early-bound and there is no 'dynamic' in sight:

using Microsoft.Office.Interop.Excel;

class Program
{
    static void Main(string[] args)
    {
        Application app = new Application();
        app.Visible = true;

        Workbook wb = app.Workbooks.Add();
        wb.Sheets[1].Name = "FirstSheet"; 
        //        ^^^^ Compiler error under .NET Core 3.0 as wb.Sheets[...] returns "object"
        //             Under C# 4 it would have been compiled and run as 'dynamic'
    }
}

This will be a show-stopper for moving a substantial class of Windows applications to .NET Core 3.0.

Please keep this issue open and high on your priority list for a future release.

@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Status of Dynamic against COM objects not working? Dynamic keyword not working against COM objects Aug 6, 2019
@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Aug 6, 2019

cc @jaredpar for FYI. I now realize this issue should be in corefx (dynamic COM binder), but keeping it here since I already moved it once - boo.

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Aug 6, 2019

Specific error message for COM and dynamic PR: dotnet/corefx#40075

@sagebind
Copy link

sagebind commented Sep 24, 2019

Here's a class I am using currently as a workaround for this:

using System;
using System.Dynamic;
using System.Reflection;

// A small wrapper around COM interop to make it more easy to use.
private class COMObject : DynamicObject
{
    private readonly object instance;

    public static COMObject CreateObject(string progID)
    {
        return new COMObject(Activator.CreateInstance(Type.GetTypeFromProgID(progID, true)));
    }

    public COMObject(object instance)
    {
        this.instance = instance;
    }

    public override bool TryGetMember(GetMemberBinder binder, out object result)
    {
        result = instance.GetType().InvokeMember(
            binder.Name,
            BindingFlags.GetProperty,
            Type.DefaultBinder,
            instance,
            new object[] { }
        );
        return true;
    }

    public override bool TrySetMember(SetMemberBinder binder, object value)
    {
        instance.GetType().InvokeMember(
            binder.Name,
            BindingFlags.SetProperty,
            Type.DefaultBinder,
            instance,
            new object[] { value }
        );
        return true;
    }

    public override bool TryInvokeMember(InvokeMemberBinder binder, object[] args, out object result)
    {
        result = instance.GetType().InvokeMember(
            binder.Name,
            BindingFlags.InvokeMethod,
            Type.DefaultBinder,
            instance,
            args
        );
        return true;
    }
}

And an example using it (in this case automating Photoshop):

dynamic application = COMObject.CreateObject("Photoshop.Application");
application.DisplayDialogs = 3; // transparent dynamic dispatch

@govert
Copy link
Contributor

govert commented Dec 3, 2019

Here are two references from the C# 4.0 release in 2009 that describe the features implemented through dynamic binding support for COM and indexed properties in C# for COM libraries:

As far as I know it is still not possible to explicitly create indexed properties in C#, which makes it hard to create a library that works the same as a PIA when referenced (thus implementing the dynamic binder support in user code as a workaround).

It really would help to light up these COM interop features again for .NET 5.

@OJacot-Descombes
Copy link

OJacot-Descombes commented Jan 25, 2020

@sagebind: I extended your helper class by wrapping the returned objects recursively, if they are not primitive types.

private static object WrapIfRequired(object obj)
{
    if (obj != null && !obj.GetType().IsPrimitive) {
        return new COMObject(obj);
    }
    return obj;
}

I then apply this to the result (result = WrapIfRequired(result);) in TryGetMember and TryInvokeMember. It saves you from having to wrap returned objects manually.


Btw.: When dynamic was introduced in C# 4.0, the primary use case mentioned was to make it simpler to talk to COM objects like Word.Application. So, it seems logical to port it to .NET Core.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@elinor-fung
Copy link
Member

elinor-fung commented Feb 13, 2020

@govert we are looking into getting this support in .NET 5 now, but I'm having some trouble repro-ing the compilation error you pointed out in #12587 (comment)

I took the sample at https://github.com/dotnet/samples/tree/master/core/extensions/ExcelDemo and added workbook.Sheets[1].Name = "FirstSheet";. It compiled with no errors (failed at run time like other scenarios needing the dynamic keyword + COM object support). What am I missing here?

@govert
Copy link
Contributor

govert commented Feb 13, 2020

@elinor-fung I'm very glad to hear you're looking at this - thank you.

I have retraced my steps to the compile error, and compare with the ExcelDemo sample project you point to. It seems there is a difference on how the PIA assembly is referenced in the project file. In my test I made a new project and added the COM reference to the "Microsoft Excel 16.0 Object Library" in Visual Studio. It works except for the IDispatch -> dynamic conversions, so leads to the compile error in that line. I now see in the ExcelDemo Readme.md that adding the COM reference in Visual Studio is not currently supported (I'm using VS 16.4.5 / .NET Core 3.1.1):

Note Adding COM references to .NET Core projects from Visual Studio is not currently supported. The workaround is to create a .NET Framework project, add the COM references, and then copy the relevant COMReference elements in the project. See ExcelDemo.csproj for further details.

The result of adding the COM reference in Visual Studio looked like this in the .csproj:

    <COMReference Include="Microsoft.Office.Excel.dll">
      <Guid>00020813-0000-0000-c000-000000000046</Guid>
      <VersionMajor>1</VersionMajor>
      <VersionMinor>9</VersionMinor>
      <WrapperTool>tlbimp</WrapperTool>
      <Lcid>0</Lcid>
      <Isolated>false</Isolated>
    </COMReference>

While the ExcelDemo sample project has the same reference looking like this:

    <COMReference Include="Microsoft.Office.Interop.Excel">
      <Guid>{00020813-0000-0000-C000-000000000046}</Guid>
      <VersionMajor>1</VersionMajor>
      <VersionMinor>9</VersionMinor>
      <Lcid>0</Lcid>
      <WrapperTool>primary</WrapperTool>
      <Isolated>False</Isolated>
      <EmbedInteropTypes>True</EmbedInteropTypes>
    </COMReference>

With the ExcelDemo-style reference, the compiler errors for this go away as you found and the types correctly show the IDispatch -> dynamic conversion, including indexed properties. Then at runtime I get the expected RuntimeBinderException that you're working on.

I experimented a bit, and the only issue is that EmbedInteropTypes is not set by default (the WrapperType does not seem to matter).
As far as I can see, that has the same effect as with the .NET Framework, which only supports the IDispatch->dynamic magic with the Embed Interop Types option.

So I think the compiler problem I report is really the known (small) tooling issue with COM references in Visual Studio / new-style .csproj files.

Ideally the tooling should be fixed to have this work (i.e. recognize as a COM reference and add the EmbedInteropTypes: true) for the case of a PIA embedded in a NUGet package too (this works fine for .NET Framework):

  <ItemGroup>
    <PackageReference Include="Microsoft.Office.Interop.Excel" Version="15.0.4795.1000" />
  </ItemGroup>

(Microsoft.Office.Interop.Excel is an unoffial NuGet package that contains the Excel 2013 PIA assembly.) Currently this package reference in .NET Core results in the same compiler error, but without the option to fix with a property on the reference in the UI or .csproj.

I should say that all these tooling issues are secondary to the internal runtime support for the COM binder. Maybe you can help report these to the right teams.

@zhusheping
Copy link

zhusheping commented Feb 20, 2020

Here's a class I am using currently as a workaround for this:

using System;
using System.Dynamic;
using System.Reflection;

// A small wrapper around COM interop to make it more easy to use.
private class COMObject : DynamicObject
{
    private readonly object instance;

    public static COMObject CreateObject(string progID)
    {
        return new COMObject(Activator.CreateInstance(Type.GetTypeFromProgID(progID, true)));
    }

    public COMObject(object instance)
    {
        this.instance = instance;
    }

    public override bool TryGetMember(GetMemberBinder binder, out object result)
    {
        result = instance.GetType().InvokeMember(
            binder.Name,
            BindingFlags.GetProperty,
            Type.DefaultBinder,
            instance,
            new object[] { }
        );
        return true;
    }

    public override bool TrySetMember(SetMemberBinder binder, object value)
    {
        instance.GetType().InvokeMember(
            binder.Name,
            BindingFlags.SetProperty,
            Type.DefaultBinder,
            instance,
            new object[] { value }
        );
        return true;
    }

    public override bool TryInvokeMember(InvokeMemberBinder binder, object[] args, out object result)
    {
        result = instance.GetType().InvokeMember(
            binder.Name,
            BindingFlags.InvokeMethod,
            Type.DefaultBinder,
            instance,
            args
        );
        return true;
    }
}

And an example using it (in this case automating Photoshop):

dynamic application = COMObject.CreateObject("Photoshop.Application");
application.DisplayDialogs = 3; // transparent dynamic dispatch

Property with indexer not work.such as:
wb.Sheets[1].Name = "FirstSheet";

@elinor-fung
Copy link
Member

elinor-fung commented Mar 5, 2020

Thanks a lot for the details @govert. I think these are the issues tracking the tooling problems you hit:

For the PackageReference issue though, it would require the package itself to be built in a way that aligns with the embed folder structure. Otherwise, there is the workaround and sample called out in NuGet/Home#2365 for adding a target to update the reference to embed interop types.

@lauxjpn
Copy link

lauxjpn commented Mar 12, 2020

@OJacot-Descombes @sagebind I took your guys work and added support to use ComObjects as values in set methods:

public override bool TrySetMember(SetMemberBinder binder, object value)
{
    _instance.GetType()
        .InvokeMember(
            binder.Name,
            BindingFlags.SetProperty,
            Type.DefaultBinder,
            _instance,
            new[]
            {
                value is ComObject comObject
                    ? comObject._instance
                    : value
            }
        );
    return true;
}

@zhusheping Implementing a general approach to indexed properties is not trivial. If you can get away with a more specific implementation, it could be as simple as the following:

public override bool TryGetIndex(GetIndexBinder binder, object[] indexes, out object result)
{
    result = WrapIfRequired(
        _instance.GetType()
            .InvokeMember(
                "Item",
                BindingFlags.GetProperty,
                Type.DefaultBinder,
                _instance,
                indexes
            ));
    return true;
}

@sagebind Are reference counts of returned objects being managed by the CLR, or do we need to track and release them ourselves (probably the later).

@smezger
Copy link

smezger commented Jul 14, 2020

@OJacot-Descombes @sagebind @lauxjpn To make it even more complete and support the invokation of members with COMObjects as arguments, I've extended the TryInvokeMember method accordingly.

  public override bool TryInvokeMember(InvokeMemberBinder binder, object[] args, out object result)
  {
      for (int i = 0; i < args.Length; i++)
      {
        if(args[i] is COMObject co)
          args[i] = co.instance;
      }
      result = instance.GetType().InvokeMember(
          binder.Name,
          BindingFlags.InvokeMethod,
          Type.DefaultBinder,
          instance,
          args
      );
      result = WrapIfRequired(result);
      return true;
}

@lauxjpn
Copy link

lauxjpn commented Aug 6, 2020

While we are waiting for the official .NET 5 release for official support, I improved the WrapIfRequired() method:

private static object WrapIfRequired(object obj)
    => obj != null &&
       obj.GetType().IsCOMObject
        ? new ComObject(obj)
        : obj;

@sebgod
Copy link

sebgod commented Sep 24, 2020

If anyone is coming from a search engine to this issue, I've tried to copy and paste this together into a coherent solution:

class COMObject : DynamicObject, IDisposable
{
    private object _instance;

    public static COMObject CreateObject(string progID)
        => new COMObject(Activator.CreateInstance(Type.GetTypeFromProgID(progID, true)));

    public COMObject(object instance)
    {
        if (instance is null)
        {
            throw new ArgumentNullException(nameof(instance));
        }
        if (!instance.GetType().IsCOMObject)
        {
            throw new ArgumentException("Object must be a COM object", nameof(instance));
        }
        _instance = instance;
    }

    public override bool TryGetMember(GetMemberBinder binder, out object result)
    {
        result = Unwrap().GetType().InvokeMember(
            binder.Name,
            BindingFlags.GetProperty,
            Type.DefaultBinder,
            Unwrap(),
            new object[] { }
        );
        return true;
    }

    public override bool TrySetMember(SetMemberBinder binder, object value)
    {
        Unwrap().GetType().InvokeMember(
            binder.Name,
            BindingFlags.SetProperty,
            Type.DefaultBinder,
            Unwrap(),
            new object[] { WrapIfRequired(value) }
        );
        return true;
    }

    public override bool TryInvokeMember(InvokeMemberBinder binder, object[] args, out object result)
    {
        for (int i = 0; i < args.Length; i++)
        {
            if(args[i] is COMObject co)
            {
                args[i] = co.Unwrap();
            }
        }
        result = Unwrap().GetType().InvokeMember(
            binder.Name,
            BindingFlags.InvokeMethod,
            Type.DefaultBinder,
            Unwrap(),
            args
        );
        result = WrapIfRequired(result);
        return true;
    }

    public override bool TryGetIndex(GetIndexBinder binder, object[] indexes, out object result)
    {
        result = WrapIfRequired(
            Unwrap().GetType()
                .InvokeMember(
                    "Item",
                    BindingFlags.GetProperty,
                    Type.DefaultBinder,
                    Unwrap(),
                    indexes
                ));
        return true;
    }

    private object Unwrap()
        => _instance ?? throw new ObjectDisposedException(nameof(_instance));

    private static object WrapIfRequired(object obj)
        => obj?.GetType()?.IsCOMObject == true ? new COMObject(obj) : obj;

    public void Dispose()
    {
        // The RCW is a .NET object and cannot be released from the finalizer,
        // because it might not exist anymore.
        var toBeDisposed = Interlocked.Exchange(ref _instance, null);
        if (toBeDisposed != null)
        {
            Marshal.ReleaseComObject(toBeDisposed);
            GC.SuppressFinalize(this);
        }
    }
}

@lauxjpn
Copy link

lauxjpn commented Sep 24, 2020

If anyone is coming from a search engine to this issue, I've tried to copy and paste this together into a coherent solution:

Very nice! For completeness, I also link to our implementation.

@sebgod
Copy link

sebgod commented Sep 24, 2020

@lauxjpn , nice implemtation, IDispose support comes in handy. I've now updated the code to also dispose and protect against double dispose, and calling the disposed COM object (via an Unwrap method), thanks!

@jzabroski
Copy link
Contributor

jzabroski commented Oct 30, 2020

@AaronRobinsonMSFT This issue is closed but I don't see a resolution. I see you marked it as done but I can't track back to what the resolution was. #12587 (comment)

In my case, I am frankly just trying to figure out the right place to file bugs for the dynamic keyword, as well as cross-reference old Microsoft Connect DLR bugs I see mentioned on StackOverflow that seem related to a problem I'm experiencing.

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Oct 30, 2020

@jzabroski @elinor-fung added this support with #33060. The link is farther above.

@msftbot msftbot bot locked as resolved and limited conversation to collaborators Dec 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.