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

Framework resolution should perform basic validation of frameworks #48180

Closed
vitek-karas opened this issue Feb 11, 2021 · 2 comments · Fixed by #90035
Closed

Framework resolution should perform basic validation of frameworks #48180

vitek-karas opened this issue Feb 11, 2021 · 2 comments · Fixed by #90035
Milestone

Comments

@vitek-karas
Copy link
Member

vitek-karas commented Feb 11, 2021

The framework resolution in hosting layer which finds the right version/location of for example Microsoft.NETCore.App framework relies solely on directory enumeration. If there's a directory with the right version number it will be selected. If the framework in that directory is somehow corrupted, the app will fail to start - there's no attempt to fallback to another version of the framework which might not be corrupted.

Based on feedback there are cases where uninstalling a framework sometimes leaves behind an empty directory. If this happens apps will fail to run, even though there's a perfectly valid version available. It's unclear what causes the empty directory.

The validation would consist of checking if the framework's .deps.json is present (the host will fail to load such framework anyway). If the file is not there, host will ignore the framework and move on to the next candidate version - effectively behaving as if that framework doesn't exist.

This change of behavior would apply to all framework resolution functionality, so framework resolution for running apps, component loading/hosting, dotnet --info and similar enumeration APIs.

It might make sense to apply similar logic to SDK versions, in that case the presence of dotnet.dll might used as the check (since host requires that file to exist in order to run any SDK commands).

Note that this is "a change" in that there will be cases where apps failed to run and they will run but on a different version. This sounds like a positive change (just wanted to be explicit about it).

The performance impact of this change should be negligible (if any at all).

This has been already discussed in multiple places: #46128 (comment) and dotnet/sdk#15021 (comment).

Similar validation is also part of the SDK CLI proposal in dotnet/sdk#15021

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Feb 11, 2021
@ghost
Copy link

ghost commented Feb 11, 2021

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

Issue Details

The framework resolution in hosting layer which finds the right version/location of for example Microsoft.NETCore.App framework relies solely on directory enumeration. If there's a directory with the right version number it will be selected. If the framework in that directory is somehow corrupted, the app will fail to start - there's no attempt to fallback to another version of the framework which might not be corrupted.

Based on feedback there are cases where uninstalling a framework sometimes leaves behind an empty directory. If this happens apps will fail to run, even though there's a perfectly valid version available. It's unclear what causes the empty directory.

The validation would consist of checking if the framework's .deps.json is present (the host will fail to load such framework anyway). If the file is not there, host will ignore the framework and move on to the next candidate version - effectively behaving as if that framework doesn't exist.

This change of behavior would apply to all framework resolution functionality, so framework resolution for running apps, component loading/hosting, dotnet --info and similar enumeration APIs.

It might make sense to apply similar logic to SDK versions, in that case the presence of dotnet.dll might used as the check (since host requires that file to exist in order to run any SDK commands).

Note that this is "a change" in that there will be cases where apps failed to run and they will run but on a different version. This sounds like a positive change (just wanted to be explicit about it).

The performance impact of this change should be negligible (if any at all).

Author: vitek-karas
Assignees: -
Labels:

area-Host

Milestone: -

@agocke
Copy link
Member

agocke commented Jul 25, 2023

This feels like working around corruption of the SDK directory. I'd prefer we just find and fix that.

@agocke agocke closed this as completed Jul 25, 2023
@ghost ghost added in-pr There is an active PR which will close this issue when it is merged and removed in-pr There is an active PR which will close this issue when it is merged labels Aug 4, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants