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

Enable loading COM component in default ALC via runtime config setting #79026

Merged
merged 5 commits into from
Dec 2, 2022

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Nov 30, 2022

Allow COM components to opt-in to being loaded in the default ALC:

  • comhost checks for the System.Runtime.InteropServices.COM.LoadComponentInDefaultContext property and uses new functions on ComActivator, loading into the default context if the property is set to true
  • Default behaviour remains loading into an isolated context
  • Fall back to using ComActivator functions that always load into an isolated context if new functions aren't found and loading in an isolated context
    • For .NET 8, we shouldn't actually need to do this, but if we want to try backporting we would.

Resolves #66013

cc @AaronRobinsonMSFT

@ghost
Copy link

ghost commented Nov 30, 2022

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

Allow COM components to opt-in to being loaded in the default ALC:

  • comhost checks for the System.Runtime.InteropServices.COM.LoadComponentInDefaultContext property and uses new functions on ComActivator, loading into the default context if the property is set to true
  • Default behaviour remains loading into an isolated context
  • Fall back to using ComActivator functions that always load into an isolated context if new functions aren't found and loading in an isolated context
    • For .NET 8, we shouldn't actually need to do this, but if we want to try backporting we would.

cc @AaronRobinsonMSFT

Author: elinor-fung
Assignees: -
Labels:

area-Host

Milestone: -

Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

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

I like this approach.

Are we going to require people to use RuntimeHostConfigurationOption to specify the new property or do you think we should introduce a new property? (I'm split on this myself)

src/native/corehost/comhost/comhost.cpp Outdated Show resolved Hide resolved
src/native/corehost/comhost/comhost.cpp Outdated Show resolved Hide resolved
@elinor-fung
Copy link
Member Author

Are we going to require people to use RuntimeHostConfigurationOption to specify the new property or do you think we should introduce a new property? (I'm split on this myself)

I was planning on just going with RuntimeHostConfigurationOption (and will update our COM sample to show it). It seems a bit too specific to me for a first-class property.

@elinor-fung
Copy link
Member Author

Failure is #78778

@elinor-fung elinor-fung merged commit 1abc433 into dotnet:main Dec 2, 2022
@elinor-fung elinor-fung deleted the com-alc branch December 2, 2022 17:47
@ghost ghost locked as resolved and limited conversation to collaborators Jan 1, 2023
@elinor-fung
Copy link
Member Author

/backport to release/7.0-staging

@github-actions github-actions bot unlocked this conversation Apr 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2023

Started backporting to release/7.0-staging: https://github.com/dotnet/runtime/actions/runs/4623484015

@elinor-fung
Copy link
Member Author

/backport to release/6.0-staging

@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2023

Started backporting to release/6.0-staging: https://github.com/dotnet/runtime/actions/runs/4623491666

@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2023

@elinor-fung backporting to release/7.0-staging failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Enable loading COM component in default ALC via runtime config setting
Using index info to reconstruct a base tree...
M	src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivationContextInternal.cs
M	src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivator.cs
M	src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.LibraryBuild.xml
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.LibraryBuild.xml
CONFLICT (content): Merge conflict in src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.LibraryBuild.xml
Auto-merging src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivator.cs
Auto-merging src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivationContextInternal.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Enable loading COM component in default ALC via runtime config setting
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2023

@elinor-fung an error occurred while backporting to release/7.0-staging, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 5, 2023
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.

COMHost/ComActivator without separate AssemblyLoadContext
3 participants