Skip to content

Revert "Partially revert PR #125326: revert ComActivator/LicenseInter…#126141

Open
AaronRobinsonMSFT wants to merge 4 commits intodotnet:mainfrom
AaronRobinsonMSFT:revert_revert_comactivator_license
Open

Revert "Partially revert PR #125326: revert ComActivator/LicenseInter…#126141
AaronRobinsonMSFT wants to merge 4 commits intodotnet:mainfrom
AaronRobinsonMSFT:revert_revert_comactivator_license

Conversation

@AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Mar 26, 2026

…opProxy changes (#125706)"

This reverts commit 3e79783.

This commit also does the following:

  • Fix test bug in LicenseTesting.h
  • Rewrite BSTRHolder to a simpler form
  • Remove layers in the managed site
  • Use the BSTR marshaller instead of Marshal class helpers
  • Remove unused signatures

Contributes to #123864

…eInteropProxy changes (dotnet#125706)"

This reverts commit 3e79783.

This commit also does the following:
- Fix test bug in LicenseTesting.h
- Rewrite BSTRHolder to a simpler form
- Remove layers in the managed site
- Use the BSTR marshaller instead of Marshal class helpers
- Remove unused signatures
@dotnet-policy-service
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 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 reverts a prior partial revert affecting COM licensing activation, and refactors the CoreCLR↔managed licensing glue to use the [UnmanagedCallersOnly] calling pattern and BSTR marshalling while also fixing a native test ownership bug.

Changes:

  • Fix BSTR ownership in the COM native test server (LicenseTesting now copies the incoming license BSTR before freeing).
  • Switch COM licensing activation in runtimecallablewrapper.cpp from MethodDescCallSite/manual ARG_SLOT plumbing to UnmanagedCallersOnlyCaller, updating the managed LicenseInteropProxy entrypoints accordingly.
  • Introduce a simplified BSTRHolder RAII type in holder.h and remove the older utilcode.h typedef; remove some metasig entries and unused signatures.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/tests/Interop/COM/NativeServer/LicenseTesting.h Fixes BSTR ownership by copying the incoming license string before destructor frees it.
src/tests/Interop/COM/NETClients/Licensing/Program.cs Renames a helper to DesignTime and updates the corresponding logging/call sites.
src/coreclr/vm/runtimecallablewrapper.cpp Reworks licensing activation to call managed helpers via UnmanagedCallersOnlyCaller and uses BSTRHolder.
src/coreclr/vm/metasig.h Removes several COM-related metasig definitions and one duplicate RetObj signature entry.
src/coreclr/vm/interoputil.cpp Updates BSTRHolder initialization style for a temporary BSTR allocation.
src/coreclr/vm/corelib.h Changes LICENSE_INTEROP_PROXY binder method signatures to NoSig for UCO usage.
src/coreclr/utilcode/comex.cpp Updates to default-constructed BSTRHolder usage for IErrorInfo::GetDescription.
src/coreclr/inc/utilcode.h Removes the old util::BSTRHolder typedef and its helper free function.
src/coreclr/inc/holder.h Adds a new BSTRHolder RAII class under FEATURE_COMINTEROP.
src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivator.cs Adds [UnmanagedCallersOnly] entrypoints for LicenseInteropProxy and switches to BStrStringMarshaller for BSTR conversion.

Copilot AI review requested due to automatic review settings March 26, 2026 17:53
Copy link
Contributor

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/tests/Interop/COM/NativeServer/LicenseTesting.h:35

  • The constructor now allocates a new BSTR before checking s_DenyLicense, but if s_DenyLicense is true the constructor throws and the destructor won’t run, leaking the newly allocated _lic. Consider performing the deny check before allocating, or using a temporary holder/RAII (allocate into a local, then assign to _lic only after the throw path is ruled out).
    LicenseTesting(_In_opt_ BSTR lic)
        : _lic{ TP_SysAllocString(lic) }
    {
        if (s_DenyLicense)
            throw CLASS_E_NOTLICENSED;
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants