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

Roll forward improvements for COM #3434

Closed
vitek-karas opened this issue Jan 31, 2019 · 16 comments
Closed

Roll forward improvements for COM #3434

vitek-karas opened this issue Jan 31, 2019 · 16 comments
Assignees
Labels
area-Host enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@vitek-karas
Copy link
Member

Scenarios

  • COM components - these are loaded into already running process

    • Process with no CoreCLR - need to determine the FX versions to load and start the CLR with - fixed for all future components/hosted code in this process.
    • Process with already loaded CoreCLR - the component needs to run on the given CLR and the associated frameworks - if it can't it fails load.
    • Want to avoid failures due to FX version mismatches as much as possible, while maintaining compatibility.
  • Apps which plan on hosting COM components

    • The goal is to avoid failures due to FX version mismatches between the app and the components it loads, while maintaining compatibility.
    • .NET Core apps should have the ability to pick the best FX versions to fulfill the goal above

Assumptions

  • .NET Core is going to be backward compatible for the most part even across major versions. For example .NET Core 3 should be able to run almost all apps built for 2.*. Note that this is already basically the case for libraries (classlibs or NuGet packages are automatically rolled forward to the framework used by the app they're part of). COM components will work effectively the same way.
  • It's preferable to allow code to run with the risk of it breaking due to breaking changes in .NET Core, over failing early and effectively trying to avoid breaks due to breaking changes.

Solution

When loading a component into existing .NET Core process the component has to use the existing CLR/Frameworks. But if the component is being loaded into a native process which doesn't have a CLR yet, it needs to start one. In this case it's important to pick the right version of the CLR (and frameworks) to start, as everything else done by the process will have to use that version.

In order to get the highest success rate of activation it's necessary to ensure that the CLR (and frameworks) in the process fulfills the requirements of the most components. Ultimately there's no 100% solution as there could be components which simply can't work together.
With the assumptions above, the highest success rate will be achieved by running the latest available runtime/framework on the machine. This means that we can't use the existing framework resolution logic because that tries to load the closest available version.

This new behavior would be needed by both

  • The COM components, since if they're the first to load CoreCLR into the process, their .runtimeconfig.json will be used to do so, and thus the new algorithm needs to be used on them.
  • The .NET Core apps which are planning to load COM components. They start the runtime via their .runtimeconfig.json and thus might want this new behavior as well.

Proposal

Add roll-forward-to-latest setting to the .runtimeconfig.json properties (globally and for completeness per-framework as well).
0 - don't roll forward - default. This is the current behavior and would be left as is.
1 - latest minor - roll forward to the latest minor version available on the machine.
2 - latest major.minor - roll forward to the latest major and minor version available on the machine.

A new setting is being proposed which will cover both the existing roll-forward-on-no-candidate-fx and the new one roll-forward-to-latest. See dotnet/designs#51.

Ease of use

An MSBuild property will be added to set this value in the project file.
For COM components, (when UseCom is set), the default value of that property will be set to roll forward to latest major. But the default would only be applied in MSBuild, the value would still be explicitly written to the .runtimeconfig.json in that case.

Apps (like the sample above of .NET Core app loading COM components) would be able to set this as well via explicit action.

@vitek-karas vitek-karas self-assigned this Jan 31, 2019
@vitek-karas
Copy link
Member Author

@nguerrera
Copy link
Contributor

Nit: The existing setting in runtimeconfig is spelled rollForwardOnNoCandidateFx so I assume this should be rollForwardToLatest, not roll-forward-to-latest?

@vitek-karas
Copy link
Member Author

@nguerrera correct (for some reason I was thinking about command line syntax)

@jeffschwMSFT
Copy link
Member

lgtm

@sdmaclea
Copy link
Contributor

Isn't 0 really latest patch.
Do we need a 3 latest prerelease at least for internal uses?

@AaronRobinsonMSFT
Copy link
Member

This seems like a reasonable versioning story. For confirmation, this new property would now be added to all runtimeconfig.json files right? The only difference with COM is that the value would be 2 instead of 0?

@sdmaclea
Copy link
Contributor

But the default would only be applied in MSBuild, the value would still be explicitly written to the .runtimeconfig.json in that case.

I think I would prefer a default behavior which didn't require a .runtimeconfig.json. COM components always use the latest installed runtime unless specified by .runtimeconfig.json.

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Jan 31, 2019

I think I would prefer a default behavior which didn't require a .runtimeconfig.json.

Do you mean the actual existence of the file or the new property? Not having a file at all is a non-starter because we need to know which framework to use and making that optional is going to make it an ambiguous scenario with the apphost/standalone scenario - no runtimeconfig.json means apphost/standalone.

@sdmaclea
Copy link
Contributor

Not having a file at all is a non-starter because we need to know which framework to use

It was a non-starter in the original discussion, but now it is not. The always use the latest installed runtime default policy for COM makes it no longer necessary.

ambiguous scenario with the apphost/standalone scenario - no runtimeconfig.json means apphost/standalone.

Not sure that assumption is necessary. Not sure why it would matter:

  • The COM component won't be confused.
  • The COM client shouldn't be confused. (Doesn't it activate using GUIDs ?)
  • The apphost won't be confused.
  • The user wouldn't be any less confused if runtimeconfig was present

@jeffschwMSFT
Copy link
Member

I am on the fence if COM components should have this behavior by default. I am happy we are heading down the path of adding the flag, and we can make the call on which value COM components will have by default.

@AaronRobinsonMSFT
Copy link
Member

@sdmaclea Which shared framework does the COM server use? We have ASP.NET, or APP? how do we decide in this case? And what about other future shared frameworks? Right now we require runtimeconfig.json in all scenarios except apphost and I think that is the right thing because in the apphost scenario the framework is adjacent to the apphost. A COM server will never have a self-contained framework, so we follow those rules. Having defaults that magically assume x, y, or z is annoying and hard to reason about. Having an explicit runtimeconfig.json makes it obvious what that this COM server has requirements. We can definitely get into cases where the runtimeconfig.json isn't respected since a CoreCLR instance is already loaded, but that is a different case because the app already defined those settings.

@jeffschwMSFT Which behavior are you referring to?

@vitek-karas
Copy link
Member Author

The proposal was that leaving the setting out (like existing files) would mean 0. And thus normal apps and so on would not write the setting into the file even in 3.0. COM components would have piece of code in MSBuild which would by default inject the setting with value 2.
The runtime would not change behavior based on activation path. While I agree it would work, I'm not sure I like this magical behavior. I am much more in favor of a consistent behavior across all entrypoints. Yes, it does require COM components to be explicit about this, but I don't think people will be playing with the runtimeconfig by hand - they'll do it through the project file, which will do this by default.

@jeffschwMSFT
Copy link
Member

@AaronRobinsonMSFT my comment aligns with what @vitek-karas points out.

@vitek-karas
Copy link
Member Author

vitek-karas commented Feb 4, 2019

Updated the issue to point to dotnet/designs#51 which supercedes this. I'll keep this issue open for now as it discusses COM specific scenarios.

@vitek-karas
Copy link
Member Author

The new roll forward is designed in
https://github.com/dotnet/designs/blob/master/accepted/runtime-binding.md
and then in detail in
https://github.com/dotnet/core-setup/blob/master/Documentation/design-docs/framework-version-resolution.md

COM components should use one of the LatestMinor or LatestMajor options to achieve the best possible outcome.

The only remaining work item is to implement a reasonable default.
Proposal is to set roll forward to LatestMinor by default to all projects which specify <UseComHost>true</UseComHost>.

@vitek-karas
Copy link
Member Author

The designed behavior LatestMinor has been implemented by default in dotnet/sdk#3305.

@msftgits msftgits transferred this issue from dotnet/core-setup Jan 30, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Host enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

6 participants