Fix algorithm description with the order problem solution (#5744)

Describe the ordering problem and how it is fixed in the new version.

Documentation/design-docs/framework-version-resolution.md

@@ -267,34 +267,41 @@ To maintain backward compatibility, each framework reference will also have to c
 
 ### Algorithm
 Terminology
-- `config fx references`: framework references for a single `.runtimeconfig.json`, where each reference consists of `name`, `version`, optional `rollForward` and optional `applyPatches`.
-- `newest fx references`: framework references keyed off of framework name that contain the highest framework version requested. It is used to perform "soft" roll-forwards to compatible references of the same framework name without reading the disk or performing excessive re-try (Step 7).
+- `framework reference`: consists of framework `name`, `version`, `rollForward` and optionally `applyPatches`.
+- `config fx references`: `framework references` for a single `.runtimeconfig.json`.
+- `newest fx references`: dictionary of `framework references` keyed off of framework `name` that contains the highest `version` requested and most constrained `rollForward` and `applyPatches`. It is used to perform "soft roll-forwards" to compatible references of the same framework name without reading the disk or performing excessive re-try.
 - `resolved frameworks`: a list of frameworks that have been resolved, meaning a compatible framework was found on disk.
 
 Steps
 1. Determine the `config fx references`:
    * Parse the application's `.runtimeconfig.json` `runtimeOptions.frameworks` section.
-   * Insert each framework reference into the `config fx references`.
-2. For each framework in `config fx references`:
-3. --> If the framework name is not currently in the `newest fx references` list Then add it.
-   * By doing this for all references here, before the next loop, we minimize the number of re-try attempts.
-4. For each framework reference in `config fx references`:
-5. --> If the framework is not in `resolved frameworks` Then resolve the framework reference to the actual framework on disk
-   * If the framework `name` already exists in the `newest fx references` resolve the currently processed reference with the one from the `newest fx references` (see above for the algorithm).  
+   * Insert each `framework reference` into the `config fx references`.
+2. For each `framework reference` in `config fx references`:
+3. --> If the framework `name` is not currently in the `newest fx references` list Then add it.
+   * By doing this for all `framework references` here, before the next loop, we minimize the number of re-try attempts.
+4. For each `framework reference` in `config fx references`:
+5. --> If the framework's `name` is not in `resolved frameworks` Then resolve the `framework reference` to the actual framework on disk:
+   * If the framework `name` already exists in the `newest fx references` resolve the currently processed `framework reference` with the one from the `newest fx references` (see above for the algorithm). 
    *Term "soft roll-forward" is used for this in the code*
-     * The resolution will always pick the higher `version` and will consolidate the `rollForward` and `applyPatches` settings.
-     * The resolution may fail if it's not possible to roll forward from one reference to the other.
-     * Update the `newest fx references` with the resolved reference.
-   * Probe for the framework on disk  
+     * The resolution will always pick the higher `version` and will consolidate the `rollForward` and `applyPatches` settings to the most constrained.
+     * The resolution may fail if it's not possible to roll forward from one `framework reference` to the other.
+     * Update the `newest fx references` with the resolved `framework reference` (note that this may be a combination of version and settings from the two `framework references` being considered).
+   * Probe for the framework on disk using the whole `framework reference` (which by now is the `framework reference` in `newest fx references`)
    *Term "hard resolve" is used for this in the code*
      * This follows the roll-forward rules as describe above.
    * If success add it to `resolved frameworks`
-     * Parse the `.runtimeconfig.json` of the resolved framework and create a new `config fx references`. Make a recursive call back to Step 2 with this new `config fx references`.
-6. --> ElseIf the `version` is < resolved `version` Then perform a "soft roll-forward" to the resolved framework.
+     * Parse the `.runtimeconfig.json` of the resolved framework and create a new `config fx references`. Make a recursive call back to Step 2 with these new `config fx references`.
+     * Continue with the next `framework reference` (Step 4).
+6. --> Else perform a "soft roll-forward" to the `framework reference` in `newest fx references`.
    * We may fail here if not compatible.
-7. --> Else re-start the algorithm (goto Step 1) with new/clear state except for `newest fx references` so we attempt to use the newer version next time.
+   * If the roll-forward results in a different `framework reference` than the one in `newest fx references`
+     * Update the `framework reference` in `newest fx references`
+     * Re-start the algorithm (goto Step 1) with new/clear state except for `newest fx references` so we attempt to use the newer `framework reference` next time.
+   * Else (no need to change the `newest fx references`) - use the already resolved framework and continue with the next `framework reference` (Step 4).
 
-This algorithm for resolving the various framework references assumes the **No Downgrading** best practice explained below in order to prevent loading a newer version of a framework than necessary.
+Notes on this algorithm:
+* This algorithm for resolving the various framework references assumes the **No Downgrading** best practice explained below in order to prevent loading a newer version of a framework than necessary.
+* Probing for the framework on disk never changes the `newest fx references`. This means that the `newest fx references` contains the latest effective framework reference for each framework without considering what frameworks are actually available. This is very important to avoid ordering issues. (See the **Fixing ordering issues** in the sections below.)
 
 
 ### Best practices for a `.runtimeconfig.json`
@@ -319,9 +326,84 @@ A newer version of a shared framework should keep or increase the version to ano
 By following these best practices we have optimal run-time performance (less processing and probing) and less chance of incompatible framework references.
 
 
+### Scenarios with known issues
+#### `LatestMajor` usage with multiple frameworks
+For example an app which has runtime config like this:
+```
+ASP.NET 3.0 rollForward=LatestMajor
+ThirdPartyFX 1.0 rollForward=LatestMajor
+```
+
+And now assume that both `ASP.NET 3.0` and `ThirdPartyFX 1.0` have references to `Microsoft.NETCore.App 3.0`. And then `ASP.NET 4.0` is released which has a reference to `Microsoft.NETCore.App 4.0`. The above application will break now, since it will pick `ASP.NET 4.0` and thus request `Microsoft.NETCore.App 4.0` but that reference is not compatible with the `Microsoft.NETCoreApp 3.0` reference from `ThirdPartyFX 1.0`.
+
+Right now we don't support 3rd party frameworks and all 1st party framework should ship in sync, so such a situation should not arise. At the same time it will be relatively uncommon to have apps with multiple framework references.
+
+This might be more of an issue for components (COM and such), which we will recommend to use at least `LatestMinor` if not `LatestMajor`. But again it would only happen if the component has references to more than on framework.
+
+
+
 ## Changes to existing apps
 The above proposal will impact behavior of existing apps (because framework resolution is in `hostfxr` which is global on the machine for all frameworks). This is a description of the changes as they apply to apps using either default settings, `rollForwardOnNoCandidateFx` or `applyPatches`.
 
+### Fixing ordering issues
+In 2.* the algorithm had a bug in it which caused it to resolve different version depending solely on the order of framework references. Consider this example:  
+
+`Microsoft.NETCore.App` is available on the machine with versions `2.1.1` and `2.1.2`.
+
+```
+Application
+ -> Microsoft.NETCore.App 2.1.0 rollForwardOnNoCandidateFx=0, applyPatches=false
+ -> ASP.NET 2.1.0
+ASP.NET (2.1.0)
+ -> Microsoft.NETCore.App 2.1.0 rollForwardOnNoCandidateFx=0
+```
+
+This would resolve `Microsoft.NETCore.App 2.1.1` because the reference from the app with `applyPatches=false` is hard resolved first and the reference from `ASP.NET` can soft roll forward to it.
+
+Now simply change the order of framework reference in the app
+
+```
+Application
+ -> ASP.NET 2.1.0
+ -> Microsoft.NETCore.App 2.1.0 rollForwardOnNoCandidateFx=0, applyPatches=false
+ASP.NET (2.1.0)
+ -> Microsoft.NETCore.App 2.1.0 rollForwardOnNoCandidateFx=0
+```
+
+This one would resolve `Microsoft.NETCore.App 2.1.2` because the reference in `ASP.NET` is hard resolved first and the one in the app can soft roll forward to it.
+In 2.* this is not a serious problem since `rollForwardOnNoCandidateFx=0` is used very rarely and more importantly none of the built in frameworks will specify it.
+
+In 3.0 with the addition of `LatestMinor` (and `LatestMajor`) this problem can become a real issue. Consider this example:
+
+`Microsoft.NETCore.App` is available on the machine with versions `3.1.1` and `3.2.0`.
+
+```
+Application
+ -> Microsoft.NETCore.App 3.1.0 rollForward=LatestMinor
+ -> ASP.NET 3.1.0
+ASP.NET (3.1.0)
+ -> Microsoft.NETCore.App 3.1.0 <default> (i.e. rollForward=Minor)
+```
+
+This would resolve `Microsoft.NETCore.App 3.2.0` because the reference from the app with `LatestMinor` is hard resolved first and the reference from `ASP.NET` can soft roll forward to it.
+
+Now simply change the order of framework reference in the app
+
+```
+Application
+ -> ASP.NET 3.1.0
+ -> Microsoft.NETCore.App 3.1.0 rollForward=LatestMinor
+ASP.NET (3.1.0)
+ -> Microsoft.NETCore.App 3.1.0 <default> (i.e. rollForward=Minor)
+```
+
+This one would resolve `Microsoft.NETCore.App 3.1.1` because the reference in `ASP.NET` is hard resolved first (`Minor` will pick the closest available minor version) and the one in the app can soft roll forward to it.
+
+Since the reference in standard framework (`ASP.NET` in this sample) has no settings (defaults) this can be very common. Specifically for COM or other components where we will recommend usage of `LatestMinor` or `LatestMajor` for best compatibility.
+Note that with `LatestMajor` the problem is even worse because depending on the order and available framework the references may actually fail to resolve with the old algorithm.
+
+The fixed algorithm doesn't consider the actual hard resolved framework version when computing the effective framework reference. See the algorithm description above. The outcome is that it will effectively always compute the full effective framework reference before hard resolving it and thus is not affected by ordering. The downside is that it may need to retry more often. To avoid unnecessary retries the best practices should be followed, specifically the one about not specifying unnecessary framework references. In the above sample, the app doesn't need to specify a framework reference for `Microsoft.NETCore.App` and so it should not do that.
+
 ### Roll on patches-only will now roll from release to pre-release if no release is available
 When `rollForwardOnNoCandidateFx` is disabled (set to `0` which is not the default) the existing behavior is to never roll forward to a pre-release version. If the setting is any other value (Minor/Major) it would roll forward to pre-release version if there's no available matching release version.