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

Extend ComWrappers API to better support aggregation #44446

Closed
AaronRobinsonMSFT opened this issue Nov 10, 2020 · 3 comments · Fixed by #47073
Closed

Extend ComWrappers API to better support aggregation #44446

AaronRobinsonMSFT opened this issue Nov 10, 2020 · 3 comments · Fixed by #47073
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.InteropServices
Milestone

Comments

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Nov 10, 2020

Background and Motivation

The ComWrappers API is intended to permit external developers the ability to create standalone COM interop platforms. During the shut down of the .NET 5 release it was determined the COM Aggregation scenarios are difficult to handle properly. There are two updates that would assist making the scenario less magic and clearer to describe and document.

Proposed API

Modification (1) - Add 2 new values - Aggregation to indicate aggregation and Unwrap to indicate the IntPtr should be inspected if it is a CCW first - to the CreateObjectFlags enumeration.

namespace System.Runtime.InteropServices
{
    [System.FlagsAttribute]
    public enum CreateObjectFlags
    {
        None = 0,
        TrackerObject = 1,
        UniqueInstance = 2,
+       Aggregation = 4,
+       Unwrap = 8,
    }
}

Modification (2) - Add a new API overload for ComWrappers.GetOrRegisterObjectForComInstance() that accepts an inner value. The details of what this inner value represents are found in the official COM Aggregation documentation.

namespace System.Runtime.InteropServices
{
    public abstract class ComWrappers
    {
        public object GetOrRegisterObjectForComInstance(System.IntPtr externalComObject, CreateObjectFlags flags, object wrapper);

+       public object GetOrRegisterObjectForComInstance(System.IntPtr externalComObject, CreateObjectFlags flags, object wrapper, System.IntPtr inner);
    }
}

Usage Examples

The scenario in question is non-trivial and difficult to reduce to a comprehensible example - below is an annotated attempt.

// A globally accessible ComWrappers instance.
ComWrappers cw = ...;

object createdWrapper = ...;

// Create native outer based on createdWrapper.
IntPtr outer = ...;

// Create native instance and aggregate with the above outer.
// In the aggregation scenario it is required to request the IUnknown interface.
Guid clsid = typeof(Demo).GUID;
Guid IID_IUnknown = typeof(IUnknown).GUID;
IntPtr inner = Native.CoCreateInstance(ref clsid, ref IID_IUnknown, outer);

// Retrieve the default interface from the inner.
Guid iid = typeof(IDemo).GUID;
IntPtr defInterface;
Marshal.QueryInterface(inner, ref iid, out defInterface);

// Register the created wrapper for the supplied default interface.
cw.GetOrRegisterObjectForComInstance(defInterface, CreateObjectFlags.Aggregation, createdWrapper, inner);

Alternative Designs

No alternative designs were considered.

Risks

There are few if any risks. This is simply a better surface area for a cumbersome and poorly describable scenario.

@AaronRobinsonMSFT AaronRobinsonMSFT added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.InteropServices labels Nov 10, 2020
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 6.0.0 milestone Nov 10, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Nov 10, 2020
@AaronRobinsonMSFT
Copy link
Member Author

@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Nov 10, 2020
@AaronRobinsonMSFT AaronRobinsonMSFT added this to To do in Interop-CoreCLR 6.0 via automation Nov 11, 2020
@AaronRobinsonMSFT AaronRobinsonMSFT added api-ready-for-review API is ready for review, it is NOT ready for implementation api-suggestion Early API idea and discussion, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation api-ready-for-review API is ready for review, it is NOT ready for implementation labels Nov 16, 2020
@AaronRobinsonMSFT AaronRobinsonMSFT added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Dec 5, 2020
@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jan 5, 2021
@terrajobst
Copy link
Member

terrajobst commented Jan 5, 2021

Video

  • Looks good, but we should change the order to preserve positional order between overloads.
namespace System.Runtime.InteropServices
{
    [Flags]
    public partial enum CreateObjectFlags
    {
        // None = 0,
        // TrackerObject = 1,
        // UniqueInstance = 2,
        Aggregation = 4,
        Unwrap = 8,
    }
}
namespace System.Runtime.InteropServices
{
    public abstract class ComWrappers
    {
        // Exists
        // public object GetOrRegisterObjectForComInstance(IntPtr externalComObject, CreateObjectFlags flags, object wrapper);
        public object GetOrRegisterObjectForComInstance(IntPtr externalComObject, CreateObjectFlags flags, object wrapper, IntPtr inner);
    }
}

@EatonZ
Copy link
Contributor

EatonZ commented Jan 7, 2021

Nit: Unnecessary comma after the 8.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 16, 2021
@ghost ghost closed this as completed in #47073 Jan 21, 2021
Interop-CoreCLR 6.0 automation moved this from To do to Done Jan 21, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 21, 2021
@dotnet dotnet locked as resolved and limited conversation to collaborators Feb 20, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.InteropServices
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants