winforms icon indicating copy to clipboard operation
winforms copied to clipboard

Hosting UserControl via ActiveX does not work correctly

Open arknu opened this issue 5 years ago • 44 comments

  • .NET Core Version: 5.0

  • Have you experienced this same bug with .NET Framework?: No

Problem description: I am experimenting with doing Office Addins without VSTO and using .NET 5. So far, I have succesfully manged to add ribbon buttons and I'm now working on a custom TaskPane, hosting a Winforms UserControl.

So far, so good. However, when instantiating the UserControl, an exception is thrown here: https://github.com/dotnet/winforms/blob/cb03d37fe73fb178d3c3827ad895bb3e7c50fe4c/src/System.Windows.Forms/src/System/Windows/Forms/Control.ActiveXImpl.cs#L841

_inPlaceUiWindow is null here and there is no check for that case.

Comparing to .NET Framework 4.8 reference source (https://referencesource.microsoft.com/#System.Windows.Forms/winforms/Managed/System/WinForms/Control.cs,17065), the implementation is different and I suspect that a regression has snuck in.

Expected behavior: No NullReferenceException should be thrown.

Minimal repro: You can download the project here: https://1drv.ms/u/s!AkwgkpetCvCpr4JeJeZfCGlKlHs4qA?e=PeB0So You need to manually run regsvr32 OutlookAddinNet5.comhost.dll in the bin folder and register the addin in outlook:

Windows Registry Editor Version 5.00

[HKEY_CURRENT_USER\SOFTWARE\Microsoft\Office\Outlook\Addins\OutlookAddinNet5]
"CommandLineSafe"=dword:00000001
"Description"="OutlookAddinNet5 description"
"FirendlyName"="OutlookAddinNet5 name"
"LoadBehavior"=dword:00000003

Then Open Outlook and click the [2] button added to the ribbon.

arknu avatar Dec 15 '20 11:12 arknu

I am experimenting with doing Office Addins without VSTO and using .NET 5.

Unfortunately there is no interest to support that scenario from the Office side (see e.g. #1763) so while the last statement from the WinForms team is to be willing to look into and fix bugs, you'll probably have a tough road ahead of you to get it working.

As far as Office integration support is concerned you cannot rely on compatibility of WinForms in .NET Core with .NET Framework since the Desktop Framework was using undocumented "Component Manager" integration with Office, and the Office team is not providing support to WinForms in .NET Core to support it properly (#247).

This is just FYI and no argument against fixing the problem you found.

Also, aside from above, TLB support has been dropped from COM interop in .NET Core so you'll either have to reinvent that or use Desktop Framework to generate the TLB for scenarios that need it. The new COM interop layer currently being built will most likely not support TLB out of the box either.

weltkante avatar Dec 15 '20 11:12 weltkante

@weltkante Thank you for the context, wasn't having much luck searching. The Office team really needs to wake up and realize that HTML/JS is fine for simple addins, but complex addins with actual functionality need a proper language and runtime with full system access, i.e. .NET and C#. VSTO needs a path forward, even if that is just doing raw COM addins with C#.

My impression was always that since Office addins under the covers are just COM and WinForms is just a Win32 wrapper, it should "just work", once you implement the correct interfaces and get Office to load you addin. If you can do a native addin in C++ and COM, surely WinForms should work without extra support from the Office team. As long as they support COM addins...

However, I am not sure that this bug is anything Office-specific. This appears to be a more general regression in the COM/ActiveX interop that I just happened to hit doing my Office addin experiments.

arknu avatar Dec 15 '20 11:12 arknu

However, I am not sure that this bug is anything Office-sepcific.

No, definitely not, ActiveX is separate and can be hosted outside Office as well, just wanted to inform you that you are likely to run into more (possibly subtile) problems.

surely WinForms should work without extra support from the Office team

Unfortunately WinForms in Desktop Framework integrated with Office to communicate form modality (WPF does not). The WinForms team didn't agree to remove that code (see discussions in the linked issues for their reasons), so since it isn't supported or tested in any way regressions are possible (and potentially subtile since they relate to how Office behaves when you show a modal WinForms form).

If you have the resources to see your project through that surely would help improve quality, we have this kind of Office Addins as well but currently all our .NET Core efforts are halted until custom control support is finished, so it may be a while before I get around to come back to rewrite and test our own addins.

weltkante avatar Dec 15 '20 11:12 weltkante

Thank you @weltkante. @arknu VSTO interop story is currently a grey area, even for us. That said, if you stumble across any bugs in our codebase - feel free to send fixes in :)

RussKie avatar Dec 15 '20 12:12 RussKie

@RussKie I don't believe this has anything to do with VSTO, so that label should be removed. In fact the original post explicitly states that I am not using VSTO at all. This is an ActiveX/COM interop issue that just happens to surface in Office, because that was where I was using it.

The code in question is missing null checks. Every other use of _inPlaceUiWindow in the code mentioned above has null checks, except this one. That seems like a mistake.

arknu avatar Dec 15 '20 13:12 arknu

I just had a look at the source for this in InPlaceActivate(OLEIVERB verb). There is a call to

HRESULT hr = _inPlaceUiWindow.GetWindow(&hwndParent);

at a location in the code where _inPlaceUiWindow has never been instantiated. This piece of code cannot work anywhere.

1R053 avatar Mar 01 '21 18:03 1R053

created a PR #4626 imo it was a copy & paste error when porting this code. In case this goes through, will it be backported to 5.x or should this be a separate PR?

1R053 avatar Mar 01 '21 20:03 1R053

@1R053 Thank you for creating the PR. It certainly fixes the NullReference exception for me. However, there appears to be other issues resulting in the ActiveX control not being created - at least when running in Office (via the ICTPFactory.CreateCTP method). I don't get any exceptions or details, just "The ActiveX object could not be created". However, those issues are probably best discussed in a separate issue, which I'll create once I have done some more investigation.

@RussKie Are you interested in having ActiveX hosting work correctly in .NET 6? If so, I'll be happy to work with you to test this. I have a proof-of-concept Outlook addin, which I can provide the code for, if needed. My specific scenario is Office, as stated previously. I suspect this will be the most common case these days, but I don't think any of these issues are Office-specific.

arknu avatar Mar 04 '21 09:03 arknu

@arknu Thanks - I've observed this as well. It would be great if you can assist in identifying the next issues.

1R053 avatar Mar 04 '21 09:03 1R053

after the fix I think the class type of the control cannot be identified somehow. When looking into Internal.Runtime.InteropServices.ComActivator->FindClassType there seems to be no type returned in my case

1R053 avatar Mar 04 '21 10:03 1R053

@1R053 For me it actually gets as far as calling the constructor of the UserControl now. Definitely progress. But after executing the constructor successfully, it fails. I have a feeling that there may be another copy-paste bug lurking somewhere...

arknu avatar Mar 04 '21 10:03 arknu

@arknu Can you register/unregister the comhost.dll with Core 6? I had also issues here. If it was already registered before I also had a successful instantiation but no success with ICTPFactory.CreateCTP

1R053 avatar Mar 04 '21 10:03 1R053

Are you interested in having ActiveX hosting work correctly in .NET 6? If so, I'll be happy to work with you to test this. I have a proof-of-concept Outlook addin, which I can provide the code for, if needed. My specific scenario is Office, as stated previously. I suspect this will be the most common case these days, but I don't think any of these issues are Office-specific.

Our current team is lacking in understanding how Office interop and ActiveX are used in the wild, and we're very interested in sample apps that can demonstrate real scenarios. Having these scenarios we'll help us building a test suite and identifying gaps, and may aid in cross-org discussion about the future of the interop.

I can't make any promises on the future support of VSTO and Office interop, but the more we know how our users use (and abuse :)) this area - the more we may be able to help.

RussKie avatar Mar 04 '21 10:03 RussKie

@RussKie As for my use case, I'm currently attempting a proof of concept to determine whether whether we can do an Outlook addin without VSTO, since there apparently no desire to update VSTO (which is a shame!). We currently have a fairly complex commercial VSTO-based addin, and no desire to be stuck on a dead .NET Framework with outdated libraries - and no desire to use a subpar language like JS. OfficeJS is not even remotely capable of doing what we need. So we are looking to do an addin using raw COM APIs directly from .NET. This should be possible and mostly is. The only issue is showing a task pane, which uses a WinForms UserControl hosted as an ActiveX object. So I have created a .NET 5-based COM addin that works perfectly - except for showing the task pane, where we run into various issues, as detailed above.

So if we get these issues fixed, we will be using Office interop and COM for a full-fledged commercial Outlook addin.

arknu avatar Mar 05 '21 07:03 arknu

If you have any repros to share but don't want those to be public please feel free to email me at Igor.Velikorossov at microsoft.

RussKie avatar Mar 09 '21 08:03 RussKie

@RussKie What I have is a proof of concept, so I have no issues sharing it publicly. I'll upload it to GitHub later this week and post a link here - I just need to clean it up slightly first.

arknu avatar Mar 09 '21 10:03 arknu

@RussKie As promised, I have now uploaded my proof-of-concept to a GitHub repository: https://github.com/arknu/OutlookAddinNet5. It is pretty much as simple as can be - adding a few buttons to the ribbon and attempting to show a taskpane with a UserControl. This last part being what fails.

It is currently based on .NET 5, haven't tried the .NET previews yet.

I'm running it with a private patched build of WinForms, which includes @1R053's patch for the NullReferenceException.

arknu avatar Mar 13 '21 12:03 arknu

I'll have a look

weltkante avatar Mar 13 '21 21:03 weltkante

ok, found two issues:

  • In IOleObject.GetMiscStatus this line looks suspicious. On Desktop Framework this code lives in an else-branch so I believe the check hasn't been properly inverted.

    It is not necessary to fix this to get your control loaded, but considering this is currently not reporting the correct flags to the host its probably good to fix anyways. I can imagine some of these flags not being reported could cause differences in behavior of the control.

  • your control needs to implement an IDispatch interface, see #4443 which had similar problems exposing a Form subclass to scripts. Previously you would inherit IDispatch from UserControl and perform arbitrary calls via reflection, but this has been removed because WinForms cannot provide a TLB anymore (and IMHO allowing arbitrary calls via reflection is not great anyways). You now have to explicitly define an interface instead of relying on reflection, because UserControl is no longer ComVisible on its own.

    The fix is to implement an appropriate interface for your class, it can be empty if you do not intend to call into it. You can add ComVisible, Guid and InterfaceType attributes to the interface as well if you want, but they are not necessary, its just required to be present in order to implement IDispatch, which is the default for unannotated interfaces. The important part is the ComDefaultInterface which replaces the reflection based default implementation (which no longer works) by an interface-based IDispatch implementation.

    public interface ITestTaskPane { }
    
    [ComVisible(true)]
    [Guid("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx")] // copy-paste protection, insert your own GUID here
    [ComDefaultInterface(typeof(ITestTaskPane))] // being explicit is required, reflection based IDispatch does not work
    public partial class TestTaskPane : UserControl, ITestTaskPane { ... }
    

    PS: in other ActiveX hosting contexts than Office, where your host may want to set properties on the control, you will have to add these properties to the interface.

weltkante avatar Mar 14 '21 03:03 weltkante

Can you register/unregister the comhost.dll with Core 6? I had also issues here.

@1R053 I've successfully done that while debugging the above, but I had to install the 32 bit SDK for some reason to get the addin registering in a 32 bit office installation (just building 32 bit worked via 64 bit SDK, but registering did not; I did have both 32 bit and 64 bit runtimes installed) - I wonder if these addins even register via regsvr32 on machines where the SDK is not installed in the first place?

If regsvr32 is broken without an SDK installed matching the bitness of the DLL then it probably needs to be reported on the dotnet runtime repo.

Also note that AnyCPU builds probably won't work for COM libraries, since the comhost.dll is likely to be a native DLL so you need to build twice for specific bitness to get both variations of the comhost.dll if you want to support both 32 bit and 64 bit office. For Desktop Framework this was not necessary because mscoree.dll, the equivalent of the comhost.dll, was built into the framework and available in both bitness variations out of the box.

weltkante avatar Mar 14 '21 04:03 weltkante

@weltkante Thank you for taking a look at my code. Adding the interface and the ComDefaultInterface attribute fixed the problem. I can now show a custom task pane with a WinForms UserControl: image

Regarding the other issue you mention, that does seem a bit suspicious, yes...

Regarding regsvr32 - I'll do some testing in a few VMs and report back in the coming week. I'll then file a new issue in dotnet/runtime if required.

Now that I have working code with .NET 5, I'll switch to testing with the .NET 6 preview and report back here.

arknu avatar Mar 14 '21 12:03 arknu

Thanks @weltkante - the task pane looks good now with this fix!

1R053 avatar Mar 14 '21 13:03 1R053

@weltkante @RussKie A quick question: I have now made a .NET 6 version of the proof-of-concept and I noticed something strange: I cannot load both the .NET 5 and 6 versions in Outlook at the same time. Either one alone loads fine, both both at the same time will cause the other one to fail to load. Is this supposed to be supported? The two addins have different namespaces and GUIDs, so from what I can see it should work.

arknu avatar Mar 16 '21 07:03 arknu

@arknu besides the COM namespaces - did you also change the addin name under Software/Microsoft/Office/Outlook/Addins?

1R053 avatar Mar 16 '21 08:03 1R053

Is this supposed to be supported?

Probably not, last statement I've seen about this was that it is not supported to load .NET more than once, since it uses global resources (like named mutexes or whatever) loading multiple instances of coreclr into the same process cannot work. They are aware of this restriction and considering lifting it, but I don't know what the progress is on that. If its not been worked on yet you will run into problems if more than one addin uses .NET Core, but really this is a question for the dotnet runtime team so you probably should post there to ask for the status of their support.

weltkante avatar Mar 16 '21 08:03 weltkante

@1R053 Yes, Outlook has two distinct addins: image

But the .NET 6 one has a COM runtime error when loading: image

However, if I disable the .NET 5 addin (by changing LoadBehavior to ), the .NET 6 addin loads just fine.

So, it seems to have an issue with loading multiple COM-hosting versions of .NET at the same time... Which I would consider a bug. Should I file an issue in dotnet/runtime to discuss this further?

arknu avatar Mar 16 '21 08:03 arknu

if this does not work, it would be a big issue imo. Since this would mean that two addins from two vendors that are usually not synced with respect to their .NET version would crash each other.

1R053 avatar Mar 16 '21 08:03 1R053

See for example dotnet/runtime#12018 (which is quite old) so its probably worth to ask again and present your usecase.

if this does not work, it would be a big issue imo

definitely, you'd risk your addin not being loadable by relying on .NET core if someone else gets loaded earlier

would crash each other.

I'd hope it'd just not load, not crash the Office process. If it crashes it may be worth fixing that crash independently from adding side-by-side support.

weltkante avatar Mar 16 '21 08:03 weltkante

Maybe a custom shim instantiating the runtime with a custom assembly context could work https://docs.microsoft.com/en-us/dotnet/api/system.runtime.loader.assemblyloadcontext?view=net-5.0

does somebody have an example for a custom shim with .NET Core?

1R053 avatar Mar 16 '21 08:03 1R053

Maybe a custom shim instantiating the runtime with a custom assembly context could work

No at the end of the issue I linked above they mention they have been using global resources. If they didn't change that yet it won't work at all.

You'd need to move your addin out of process to make the runtime resolve uniquely, and have a C++ COM addin as a shim (or any other language that can load side-by-side)

We actually do something very similar for different reasons, on Desktop Framework WinForms already is a shared resource and we ran into problems because other addins may have loaded the same 3rd party WinForms controls we use before us with a different global configuration.

Therefore we moved all our UI into an out-of-process COM object, keeping an in-process addin to communicate with Office and shim all calls that do UI to our out-of-process objects. This is nontrivial due to how COM behaves when crossing apartmant boundaries, especially message pumping happening to wait for results of the call requires low level workarounds (you don't want message pumping when the ribbon is being loaded for example, so we do the out-of-process COM calls on the thread pool, with special non-pumping code to wait for results, being careful that we don't have deadlocks).

If we were to port to .NET Core we could probably update that approach and make the in-process addin C++ only until .NET Core actually supports loading side-by-side (though I don't know if we would take that step, considering we prefer coding in C# where possible)

weltkante avatar Mar 16 '21 08:03 weltkante