runtime: COM: non-optional output parameters inconsistent when passing NULL

Scenario

Native Code calls HRESULT hr = p->GetValue(NULL); for a non-optional output parameter and observes the HRESULT.

Depending on how the managed COM interface was declared different things happen:

  • result GetValue()
    • always executes code and discards the result, returning S_OK
  • void GetValue(out result)
    • if result is an interface, code is not executed, E_POINTER is returned
    • if result is a primitive type, code is executed, writing the output variable causes NRE, which is then translated to E_POINTER
  • [PreserveSig] int GetValue(out result)
    • if result is an interface, code is not executed, E_POINTER is returned
    • if result is a primitive type, code is executed, writing the output variable causes NRE, which is then translated to E_POINTER
  • [PreserveSig] HRESULT GetValue(out result) with HRESULT being an enum or struct
    • if result is an interface, code is not executed, S_OK is returned
    • if result is a primitive type, code is executed, writing the output variable causes NRE, which is then ignored, returning S_OK anyways

Observed behavior

  • Variant (4) returns S_OK even though no code was executed or an exception was thrown

Expected behavior

  • Variant (4) should behave more like (2) or (3) and return E_POINTER instead of S_OK
    • do not claim S_OK when no code was executed
    • do not claim S_OK when an exception was thrown and swallowed

Impact

This is not a regression (.NET Core and Desktop Framework have the same behavior) but strongly impacts the porting efforts for WinForms, in particular writing unit tests covering failure cases while at the same time using strongly typed HRESULT return values.

  • WPF has already been using Variant (4) on Desktop and is still using it in Core
  • WinForms was using Variant (3) on Desktop and has been moved to (4) while porting to Core in order to make use of strong typing (extension methods etc.)
  • HRESULT structs or enums are likely also used in third party code

WPF is currently using HRESULT structs while WinForms is using HRESULT enums. There was discussion about moving to structs like WPF does, but as far as this issue is concerned it doesn’t seem to make a difference.

The primary questions are whether

  • this is a bug and
  • can it be fixed or does it need to remain broken for compatibility forever?

As far as I can evaluate it APIs implemented in .NET via variant (4) are inherently broken in the failure case and everyone just hopes nobody calls them with invalid arguments.

Reproduction

Scenario implemented over at https://github.com/weltkante/ComInteropIssue with a reg-free-com ATL project and corresponding Core/Desktop managed console projects. (PS: I was a bit in a hurry so the interfaces are dual not IUnknown, if thats a problem I can update the project.)

INativeAPI GetInstance()                      -> HR=00000000 Called=True  Exception=none
int GetNumber()                               -> HR=00000000 Called=True  Exception=none
void GetInstance(out INativeAPI)              -> HR=80004003 Called=False Exception=none
void GetNumber(out int)                       -> HR=80004003 Called=True  Exception=NullReferenceException
int GetInstance(out INativeAPI)               -> HR=80004003 Called=False Exception=none
int GetNumber(out int)                        -> HR=80004003 Called=True  Exception=NullReferenceException
hresult-enum GetInstance(out INativeAPI)      -> HR=00000000 Called=False Exception=none
hresult-enum GetNumber(out int)               -> HR=00000000 Called=True  Exception=NullReferenceException
hresult-struct GetInstance(out INativeAPI)    -> HR=00000000 Called=False Exception=none
hresult-struct GetNumber(out int)             -> HR=00000000 Called=True  Exception=NullReferenceException

Original discussion here: https://github.com/dotnet/winforms/pull/1932#discussion_r419770875 /cc @AaronRobinsonMSFT @JeremyKuhne @hughbe

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 16 (16 by maintainers)

Most upvoted comments

@hughbe Oh sorry. Yes you are right I believe, I am so used to fighting these issues with value types.