Runtime: ASP.NET MVC 2.0 apps don't work on 2.1 runtime

[eerhardt]

Repro Steps

1. dotnet new mvc. Make sure your app targets netcoreapp2.0.
2. dotnet build
3. Install the latest shared framework somewhere. I installed 2.1.0-preview1-25714-02
4. /path/to/dotnet --fx-version 2.1.0-preview1-25714-02 bin\Debug\netcoreapp2.0\RollForwardTest.dll

Expected results

The app should run successfully

Actual results

An error is thrown:

fail: Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingProvider[48]
      An error occurred while reading the key ring.
System.IO.FileLoadException: Could not load file or assembly 'System.Runtime.CompilerServices.Unsafe, Version=4.0.4.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)
File name: 'System.Runtime.CompilerServices.Unsafe, Version=4.0.4.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'
   at System.SpanHelpers.IndexOf[T](T& searchSpace, T value, Int32 length)
   at System.SpanExtensions.IndexOf[T](ReadOnlySpan`1 span, T value)
   at System.FixedBufferExtensions.GetFixedBufferStringLength(ReadOnlySpan`1 span)
   at System.FixedBufferExtensions.GetStringFromFixedBuffer(ReadOnlySpan`1 span)
   at System.IO.SearchResultHandler.FileSystemInfoResultHandler.IsResultIncluded(String fullPath, String userPath, WIN32_FIND_DATA& findData, FileSystemInfo& result)
   at System.IO.Win32FileSystemEnumerableIterator`1.IsResultIncluded(WIN32_FIND_DATA& findData, TSource& result)
   at System.IO.Win32FileSystemEnumerableIterator`1.CommonInit()
   at System.IO.Win32FileSystemEnumerableIterator`1..ctor(String path, String originalUserPath, String searchPattern, SearchOption searchOption, SearchResultHandler`1 resultHandler)
   at System.IO.Win32FileSystem.EnumerateFileSystemInfos(String fullPath, String searchPattern, SearchOption searchOption, SearchTarget searchTarget)
   at System.IO.DirectoryInfo.EnumerateFileSystemInfos(String searchPattern, SearchOption searchOption)
   at Microsoft.AspNetCore.DataProtection.Repositories.FileSystemXmlRepository.<GetAllElementsCore>d__11.MoveNext()
   at System.Collections.Generic.List`1.AddEnumerable(IEnumerable`1 enumerable)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Microsoft.AspNetCore.DataProtection.Repositories.FileSystemXmlRepository.GetAllElements()
   at Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager.GetAllKeys()
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingProvider.CreateCacheableKeyRingCore(DateTimeOffset now, IKey keyJustAdded)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingProvider.Microsoft.AspNetCore.DataProtection.KeyManagement.Internal.ICacheableKeyRingProvider.GetCacheableKeyRing(DateTimeOffset now)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingProvider.GetCurrentKeyRingCore(DateTime utcNow)

Notes

See related issue https://github.com/dotnet/corefx/issues/23988 for why this error is being thrown.

We will need to address this issue in the context of when 2.0 apps are run on 2.1 runtime.

/cc @Petermarcu @ericstj @weshaggard

[ericstj]

The loader policy right now is dumb: if an assembly is in the app it is preferred. Essentially the host was pushing the version resolution into the build.

When you run on a different framework than you've built against you invalidate that assumption. If you want to make it work again you've got to replay what's happening at build at runtime (conflict resolution).

Note the same problem existed back in 1.0 (though it was type=platform and nuget that was implementing the policy decision, not conflict resolution). Back then we said that if you wanted to run an application on a newer framework you'd have to recompute the deps file and republish.

[muratg]

Folks, who will be fixing this one, and is there an ETA? My understanding is that we'd like to get this in to unblock consuming 2.1 runtime builds.

cc @mikeharder @natemcmaster @pakrym @Eilon

[eerhardt]

@muratg - if the project targets netcoreapp2.1 TFM, then this scenario should work correctly.

This scenario is for the case where the project targets netcoreapp2.0, and then tries to run on the 2.1 shared framework. This is a new scenario that we haven't supported previously. Previously we only supported running on the same major/minor version of the shared framework as what the project was targeting.

So, this shouldn't be blocking ASP.NET from consuming 2.1 runtime builds.

[steveharter]

An update: this is being actively worked on.

Roll-forward cases:

• The highest assembly version wins (whether app or fx)
• If a tie, the app wins

Non roll-forward cases:

• The app version always wins
  -- If version is too low, there will likely be an assembly load exception later during coreclr initialization or runtime. This exception will help prevent mismatches by causing a hard exception at runtime during development to ensure that the developer pulls (or compiles) the newer version of that app local assembly. _Note: there may be alternate ways we should discuss to prevent mismatches at development-time such as errors during compile- or publish-time_
• No need to look at assembly versions (at least against app vs fx)

To check the assembly version

• Use the assembly version from deps file (we need to add this capability)
• If assembly version not specified in deps file, or there is no deps file, call into coreclr native methods to obtain the assembly version

[weshaggard]

If a tie, the app wins

Why would the app win? That seems like the wrong direction and likely to still cause issue for anything in the shared framework that depends on this library.

I cannot remember but is roll-forward across minor versions on by default?

-- If version is too low, there will likely be an assembly load exception later during coreclr initialization or runtime. We want to fail to ensure that the developer re-targets the app to newer runtime

Why would the re-target there app to a newer runtime in this case? It seems like they would need to update their app-local dependency to at least match what the runtime carries.

Personally I think it would be a good idea to run conflict resolution in both the roll-forward and the non-roll-forward cases. It sees like it would give folks the most consistent story. We have to remember that most people are never going to test the roll-forward cases so they will not see the conflict resolution behavior during development.

[eerhardt]

Why would the app win?

This matches the policy of the "non roll-forward" case. If all other things are equal - the app wins. This allows for more flexible patching and allows the developer to decide exactly which assembly they want to load. This is purposefully opposed to the .NET Framework policy (ex. where the GAC wins and the developer can't override it).

[weshaggard]

There is a high likelihood that we stop changing assembly versions for other reasons and in that world we are asking for trouble if the application wins ties. Perhaps if we also consider file version as well then I could be OK with ties going to the application.

[steveharter]

Why would the re-target there app to a newer runtime in this case? It seems like they would need to update their app-local dependency to at least match what the runtime carries.

I will re-word my comment. They need to pull the newer version of the fx assembly however they got it to begin with (or source code if they got it that way) because that newer version (or higher) is likely going to be required by other assemblies in the fx at runtime by coreclr. So I meant "re-target" to a newer app local assembly.

[steveharter]

Personally I think it would be a good idea to run conflict resolution in both the roll-forward and the non-roll-forward cases. It sees like it would give folks the most consistent story. We have to remember that most people are never going to test the roll-forward cases so they will not see the conflict resolution behavior during development.

The intent was to avoid silent failures mistakes during the development cycle. For the non-roll forward case, if an assembly is dropped in app local and it isn't loaded (because it has a lower version than the fx assembly) then we end up "silently" ignoring their assembly meaning whatever fixes changes they assumed in their app local copy won't be used.

If we can get an error in some other way (at compile publish) that could also work to avoid the "silent" failure mode.

[weshaggard]

I don't like the silent failure mode in either case so that seems like we should figure out a story to enable developers understand what is happening.

[jkotas]

The “dotnet” host does not deal with assembly versions. It deals with nuget package versions and I think it should stay that way.

Why can’t you do this unification based on nuget package versions? The nuget package versions are in the deps.json file already.

[eerhardt]

Why can’t you do this unification based on nuget package versions?

Because the nuget package that the assembly is in has changed. Since the package changed, the nuget package version doesn't mean anything between two different packages.

In 2.0, the System.Runtime.CompilerServices.Unsafe assembly was in the System.Runtime.CompilerServices.Unsafe nuget package, and it had version 4.4.0.

In 2.1, the System.Runtime.CompilerServices.Unsafe assembly is now in the Microsoft.NETCore.App package, and that package has version 2.1.0-....

The host can't know that Microsoft.NETCore.App v 2.1.0 should be used over System.Runtime.CompilerServices.Unsafe v 4.4.0.

[jkotas]

The manifest for Microsoft.NETCore.App framework needs to contain information that it bundles System.Runtime.CompilerServices.Unsafe nuget package now.

[eerhardt]

If by manifest, you mean the Microsoft.NETCore.App.deps.json file, it does contain information that it bundles the System.Runtime.CompilerServices.Unsafe assembly now.

However, this is not enough information because of the existing policy in .NET Core that app-local wins.

Excerpt from the above linked doc:

For example, the local app location has priority over the shared framework locations and if the same assembly exists in both locations, the coreclr will end up using the local app's copy of that assembly.

So the host sees that the same assembly is in 2 locations, and it needs to know how to resolve the conflict. Previously the policy was simple - there was an order of folders probed, and the first found assembly won. However, that policy doesn't support this scenario. There are times when app-local should win and there are times when shared framework should win.

[jkotas]

There are times when app-local should win and there are times when shared framework should win.

Right. But you do need to assembly versions for that. You should be able to implement the default policy override using nuget package versions just fine.

We do have similar default policy override implemented for security servicing already: security servicing can override the version that comes with the app. It is based on nuget package versions as well, no assembly versions.

[jkotas]

that it bundles the System.Runtime.CompilerServices.Unsafe assembly now

Then it also needs information that it bundles System.Runtime.CompilerServices.Unsafe NuGet package.

[steveharter]

If every assembly had it's own nuget package and we decomposed every metapackage into those then yes we can make some intelligent decisions based on package version. Note that coreclr has both metapackage and individual assembly packages (for target servicing), but having individual assembly packages is not a requirement and doesn't apply to other frameworks such as asp.net.

Also there are other scenarios besides the one being discussed, including those that don't use packages on the app side. The app can build their own version of a framework assembly, reference it (as an assembly reference, not a package reference), and expect it to be used (or not used, depending on the versioning policy).

Here’s the “app local wins” scenarios as I see them:

1. A locally-built framework assembly with some bug fixes enhancements that haven’t yet made it into corefx
2. A locally placed copy of a newer framework assembly, such as an OOB assembly like System.Collections.Immutable
3. A locally placed copy of an older version of a framework assembly due to an unforeseen breaking change in the official newer version (patch version)

[jkotas]

It should be a non-goal to make the roll forward work well for cases where the app has a private builds of something that needs to get unified. You cannot reason about whether the customizations make by the app are also included in the version bundled by the framework. The best we can do in this case is to print error, I think.

[steveharter]

@eerhardt @jkotas here's our offline meeting notes.

We discussed three classes of scenarios and approaches to each.

The three scenarios:
1) Framework should win. This is the current scenario as described in this issue's repro steps (assembly added to framework package is newer than the app's)
2) App should win. The app wants to bring in a newer OOB assembly like System.Collections.Immutable
3) Unknown who should win. The app has locally build framework assemblies or assembly references (not package references) but a roll-forward to a newer framework has occurred.

The proposed solutions to each:
1) Add new metadata to the framework's deps.json to include the package version so we can make decisions on which is the newer version that we should use (details TBD, but could be as simple as including the versions of some of the decomposed packages that we previously only shipped OOB, such as System.Runtime.CompilerServices.Unsafe)
2) App local wins (like today)
3) App local wins (like today). The app should be re-targeted to the newer framework version, or the older framework should be installed (the one the app targeted).

Note that scenario (2) could change into scenario (1) when the OOB assembly becomes older than the current framework's assembly during a roll-forward scenario.

[weshaggard]

@steveharter I'm trying to follow the entire discussion so I can better understand the policy. Do we have a spec written up for this? I also want to be sure that whatever we end up with here as policy matches the tooling policy, in particular the conflict resolution, and we don't end up with different semantics.

[steveharter]

@weshaggard there is no spec yet, but we do need one. I'll update this issue when I have something more.

This will likely be a preview2 feature.

[Petermarcu]

We should talk about whether some form of it can make it into preview1 as I believe other parts of the stack are expecting 2.0 apps to be able to run on 2.1.

[steveharter]

Here's approaches for preview1 timeframe: (for minor roll-forward only) in order from low to higher cost:
1) Fix the specific System.Runtime.CompilerServices.Unsafe issue by hard-coding the fallback logic for this single assembly
2) When the app has a reference to a package (not assembly) and that package contains a framework assembly, use the framework's copy of that assembly. This means there is an edge case that may break - when the app has a newer version of the packageassembly; but since that would only occur during minor roll-forward it would be very rare
3) Do it the right way by comparing package versions as described earlier; need CLI supportcoordination

[Petermarcu]

For 3, what will happen for existing apps? That information won't be in the deps.json for 2.0 apps would it?

For Preview1, we can probably live with 1 as long as the behavior compatibly rolls into what we do for Preview2.

[Petermarcu]

Actually, is the proposal that the additional metadata would be in the deps.json of 2.1 so we know it now owns up to a certain version of a package/assembly?

[gulbanana]

point 2 sounds like reinventing the GAC. i’m not looking forward to installing .net core 3.x apps onto some user’s machine and then trying to work out why my dlls aren’t being used (because they have 3.5.x, which took an OOB package and brought it into the framework..)

[jkotas]

Fix the specific System.Runtime.CompilerServices.Unsafe issue

We may rather rip out System.Runtime.CompilerServices.Unsafe from netcoreapp. It would solve other problems with this partial OOB - https://github.com/dotnet/corefx/issues/24012. The fewer partial OOBs the better. This one does not have any exchange types so there are no downsides.

[Petermarcu]

I thought it went in because of System.Memory. I would prefer things not to go in if they don't need to.

[jkotas]

The OOB version of System.Memory depends on S.R.CS.Unsafe. It was an easy choice to keep the same dependency for the inbox version. This dependency is not necessary for the inbox version and can be easily avoided.

[Petermarcu]

@KrzysztofCwalina can we remove this dependency so we don't have this specific app compat challenge for Preview1?

[jkotas]

The work required for this:

• Rename System.Runtime.CompilerServices.Unsafe in CoreLib to Internal.Runtime.CompilerServices.Unsafe and make it public. The different namespace is necessary because of there are scenarios that compile against implementation assemblies, and having type with same name in two different assemblies always break them.
• Update inbox System.Memory (and other inbox CoreFX assemblies with this dependency) to pick up Unsafe from CoreLib. For some of the other inbox CoreFX assemblies, it may be better to adjust the code to not use Unsafe instead.
• Undo the change that added System.Runtime.CompilerServices.Unsafe to netcoreapp

[KrzysztofCwalina]

So, now we are going to have duplicated APIs in two different namespaces? And what's worse the in-platform one, which majority of people will be using long term, being called "Internal", but actually being public? This is confusing and keeps adding warts to the platform.

cc: @terrajobst

[jkotas]

This just .NET Core implementation details. The Internal namespace is not going to be exposed in the .NET Core reference assemblies.

We have number of similar ones already. For example, there is a public lightweight Internal.Console in S.P.CoreLib that we use for tests or debug-only logging. Other similar duplicated implementations include Environment or ConcurrentDictionary.

The whole point of this is to prevent System.Runtime.CompilerServices.Unsafe from turning into partial OOB that will effectively freeze it.

[weshaggard]

While I'm open to the idea of duplicating the unsafe APIs in corelib and not putting the OOB inbox but are we not also going to have the same issue with System.Memory? I realize we didn't ship a stable System.Memory where we did with the Unsafe OOB but I suspect there may be folks using System.Memory in a similar fashion that we want to ensure work with 2.1 preview 1.

[jkotas]

System.Memory preview packages had number of breaking changes between different versions. I do not think we need to worry about making some random old version of System.Memory preview work on .NET Core 2.1.

[KrzysztofCwalina]

The Internal namespace is not going to be exposed in the .NET Core reference assemblies.

OK, this is much less of a wart.

[Petermarcu]

@joshfree

[joshfree]

@ahsonkhan could you pick up the Unsafe fork/rename per @jkotas advice above to unblock preview1 appcompat. This needs to be resolved in the next day or two.

[steveharter]

@gulbanana

point 2 sounds like reinventing the GAC. i’m not looking forward to installing .net core 3.x apps onto some user’s machine and then trying to work out why my dlls aren’t being used (because they have 3.5.x, which took an OOB package and brought it into the framework..)

If a 3.5 framework took in an OOB package then the framework internally depends on that version or higher, so if a lower version is found first (from the app's OOB package reference) there will be an assembly load exception whenif the framework internally tries to use the OOB, so it is likely safer to try to use the newer framework's version. It is unlikely the app has a higher version of the OOB package, because it targeted an older version of the framework (say 3.4) and is running on the new version (3.5). If the app was updated later to use a newer OOB (but didn't re-target against 3.5) then yes that would cause an issue.

[KrzysztofCwalina]

cc: @GrabYourPitchforks

[steveharter]

@Petermarcu

For 3, what will happen for existing apps? That information won't be in the deps.json for 2.0 apps would it?
For Preview1, we can probably live with 1 as long as the behavior compatibly rolls into what we do for Preview2.
Actually, is the proposal that the additional metadata would be in the deps.json of 2.1 so we know it now owns up to a certain version of a package/assembly?

The additional metadata would be in the framework's 2.1 deps.json and would consist of adding package versions. The app's deps.json would already have the OOB package versions (even in the 2.0 timeframe). So we just compare the two.

[ahsonkhan]

Rename System.Runtime.CompilerServices.Unsafe in CoreLib to Internal.Runtime.CompilerServices.Unsafe and make it public.
Update inbox System.Memory (and other inbox CoreFX assemblies with this dependency) to pick up Unsafe from CoreLib.

Should we also make sure that the APIs available in both are identical?
For instance, here are some of the APIs that System.Memory depends on (from https://github.com/dotnet/corefx/blob/master/src/System.Runtime.CompilerServices.Unsafe/ref/System.Runtime.CompilerServices.Unsafe.cs) that don't exist in mscorlib (https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Runtime/CompilerServices/Unsafe.cs)

```C#
public static System.IntPtr ByteOffset(ref T origin, ref T target) { throw null; }

public unsafe static ref T AsRef(void* source) { throw null; }

public unsafe static T Read(void* source) { throw null; }

public unsafe static void InitBlockUnaligned(void* startAddress, byte value, uint byteCount) { }

public static void CopyBlock(ref byte destination, ref byte source, uint byteCount) { }

// used in some tests
public static ref T Subtract(ref T source, int elementOffset) { throw null; }

Here are the remaining APIs that don't exist in both (that S.M doesn't rely on):
```C#
public static ref T AsRef<T>(in T source) { throw null; }

public unsafe static void Write<T>(void* destination, T value) { }

public static void InitBlock(ref byte startAddress, byte value, uint byteCount) { }
public unsafe static void InitBlock(void* startAddress, byte value, uint byteCount) { }

public unsafe static void Copy<T>(void* destination, ref T source) { }
public unsafe static void Copy<T>(ref T destination, void* source) { }

public static ref T Add<T>(ref T source, System.IntPtr elementOffset) { throw null; }

public unsafe static void* Subtract<T>(void* source, int elementOffset) { throw null; }
public static ref T Subtract<T>(ref T source, System.IntPtr elementOffset) { throw null; }

public unsafe static void CopyBlock(void* destination, void* source, uint byteCount) { }
public static void CopyBlockUnaligned(ref byte destination, ref byte source, uint byteCount) { }
public unsafe static void CopyBlockUnaligned(void* destination, void* source, uint byteCount) { }

[jkotas]

The API do not have to be identical - the public S.R.CS.Unsafe can have extra APIs. You just need to add the ones that System.Memory needs.

[jkotas]

used in some tests

The tests should still depend on the public S.R.CS.Unsafe.

[ahsonkhan]

Undo the change that added System.Runtime.CompilerServices.Unsafe to netcoreapp

What would be required to make this change?

Do we mark the IsNETCoreApp/IsUAP properties as false and remove the netcoreapp build configuration?
https://github.com/dotnet/corefx/blob/master/src/System.Runtime.CompilerServices.Unsafe/dir.props#L7
https://github.com/dotnet/corefx/blob/master/src/System.Runtime.CompilerServices.Unsafe/src/Configurations.props#L7

I don't fully understand what this property in the pkgproj does <ValidatePackageSuppression Include="TreatAsOutOfBox">:
https://github.com/dotnet/corefx/blob/master/src/System.Runtime.CompilerServices.Unsafe/pkg/System.Runtime.CompilerServices.Unsafe.pkgproj#L16

cc @JeremyKuhne, @ericstj

[weshaggard]

Yes remove the IsNETCoreApp/IsUAP properties and remove the netcoreapp build configurations. Also remove the TreatAsOutOfBox suppression once those other properties are removed.

[ahsonkhan]

@marek-safar

Flagging as a potential breaking change. FYI

[Petermarcu]

Did we verify that a 2.0 ASP.NET App can now run on 2.1? I want to make sure the end user scenario is addressed and if not, we can re-open this issue to track the next problem we find with it.

[steveharter]

I verified success with latest 2.1 preview:

D:\git\repros\mvc2>more bin\Debug\netcoreapp2.0\mvc2.runtimeconfig.json
{
  "runtimeOptions": {
    "tfm": "netcoreapp2.0",
    "framework": {
      "name": "Microsoft.NETCore.App",
      "version": "2.0.0"
    },
    "configProperties": {
      "System.GC.Server": true
    }
  }
}

D:\git\repros\mvc2>dotnet --fx-version 2.1.0-preview1-26019-03 bin\Debug\netcoreapp2.0\mvc2.dll
Hosting environment: Production
Content root path: D:\git\repros\mvc2
Now listening on: http://localhost:5000
Application started. Press Ctrl+C to shut down.

Also tested this scenario with auto roll-forward from 2.0 to 2.1 when 2.0 is not installed (with new roll-forward policy changes via https://github.com/dotnet/core-setup/issues/3469)

D:\git\repros\mvc2>dotnet --list-runtimes
Microsoft.NETCore.App 2.1.0-preview1-26019-03 [C:\Program Files\dotnet\shared]

D:\git\repros\mvc2>dotnet bin\Debug\netcoreapp2.0\mvc2.dll
Hosting environment: Production
Content root path: D:\git\repros\mvc2
Now listening on: http://localhost:5000
Application started. Press Ctrl+C to shut down.

Snippet from COREHOST_TRACE=1:

--- Resolving FX directory, name 'Microsoft.NETCore.App' version '2.0.0'
Searching FX directory in [C:\Program Files\dotnet]
Attempting FX roll forward starting from [2.0.0]
'Roll forward on no candidate fx' enabled with value [1]. Looking for the least production greater than or equal to [2.0.0]
No production greater than or equal to [2.0.0] found. Looking for the least preview greater than [2.0.0]
Found version [2.1.0-preview1-26019-03]
Applying patch roll forward from [2.1.0-preview1-26019-03]
Inspecting version... [2.1.0-preview1-26019-03]
Changing Selected FX version from [] to [C:\Program Files\dotnet\shared\Microsoft.NETCore.App\2.1.0-preview1-26019-03]
Chose FX version [C:\Program Files\dotnet\shared\Microsoft.NETCore.App\2.1.0-preview1-26019-03]