Skip to content

Implement new ComWrappers.CreateObject API #115437

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

Merged
merged 4 commits into from
May 13, 2025

Conversation

jkoritzinsky
Copy link
Member

Fixes #113622

Depends on #113907

@Copilot Copilot AI review requested due to automatic review settings May 9, 2025 22:09
@jkoritzinsky jkoritzinsky added area-System.Runtime.InteropServices blocked Issue/PR is blocked on something - see comments labels May 9, 2025
@jkoritzinsky jkoritzinsky requested review from AaronRobinsonMSFT and removed request for MichalStrehovsky and Copilot May 9, 2025 22:10
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements the new ComWrappers.CreateObject API by refactoring COM interop and tracker object management across nativeaot, interop, and runtime files.

  • Updated TrackerObjectManager and related interop methods to use a partial class and new functions (e.g. TryRegisterReferenceTrackerManager).
  • Added new overloads for GC.Collect incorporating a lowMemoryPressure parameter.
  • Refactored COM wrapper logic including removal of the deprecated ComWrappers.cs file and integration of ComWrappers.CoreCLR.cs along with associated project file updates.

Reviewed Changes

Copilot reviewed 65 out of 65 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.NativeAot.cs Refactored tracker manager functions and updated boolean handling and API signatures.
src/coreclr/nativeaot/System.Private.CoreLib/src/System/GC.NativeAot.cs Introduced new GC.Collect overload with lowMemoryPressure parameter.
src/coreclr/interop/* Updated various interop and ABI definitions to support the new ComWrappers API.
src/coreclr/interop/comwrappers.hpp, ComWrappers.CoreCLR.cs Removed deprecated COM wrappers and centralized COM interop logic using new definitions.
src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.CoreCLR.cs Added new native calls for tracker object management.
src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj Updated compile items to include new COM interop files and remove deprecated ones.
Comments suppressed due to low confidence (1)

src/coreclr/interop/inc/interoplibimports.h:24

  • The IteratorNext method now returns a bool instead of an HRESULT. Please verify that all call sites are updated to check for a false return value rather than an error HRESULT to ensure consistent error handling.
bool IteratorNext(RuntimeCallContext* runtimeContext, _Outptr_result_maybenull_ void** trackerTarget, _Outptr_result_maybenull_ InteropLib::OBJECTHANDLE* proxyObject) noexcept;

Copy link
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements a new ComWrappers.CreateObject API overload to support userState parameters, addressing #113622. Key changes include:

  • Introducing the INotWrappedObject interface and NotWrappedObject class to handle non-wrapped COM objects.
  • Adding a new CreateObject overload in TestComWrappers and updating internal ComWrappers APIs to pass along a userState parameter.
  • Updating reference and core implementation files to support the new API, along with accompanying tests.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/tests/Interop/COM/ComWrappers/Common.cs Added INotWrappedObject and NotWrappedObject for testing non-wrapped objects.
src/tests/Interop/COM/ComWrappers/API/Program.cs Introduced the userState overload support in TestComWrappers and added tests for the new behavior.
src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/StrategyBasedComWrappers.cs Added a new CreateObject override that forwards to the base method.
src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs Updated API signatures and attributes to include the new userState overload.
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs Updated internal ComWrappers API implementation to handle userState with a sentinel (NoUserState).
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CreatedWrapperFlags.cs & projitems/Strings.resx Added a new CreatedWrapperFlags type and associated string resource.
Comments suppressed due to low confidence (1)

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs:1177

  • [nitpick] Consider adding an inline comment explaining that NoUserState.Instance is used as a sentinel to signal the absence of a user state, which can improve readability and maintainability.
object? retValue = userState is NoUserState ? CreateObject(identity, flags) : CreateObject(identity, flags, userState, out wrapperFlags);

@jkoritzinsky
Copy link
Member Author

/ba-g http timeout unrelated

@jkoritzinsky jkoritzinsky merged commit 406d54a into dotnet:main May 13, 2025
146 of 150 checks passed
@jkoritzinsky jkoritzinsky deleted the createobject branch May 13, 2025 18:39
@github-actions github-actions bot locked and limited conversation to collaborators Jun 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API Proposal]: Allow the user to provide state for a ComWrappers.GetOrCreateObjectForComInstance operation
3 participants