Hot Restart pre-built bundle contains outdated outbox assemblies

[filipnavara]

Hot Restart prebuilt bundle carries couple of non-Inbox assemblies, among them System.Net.Http.Json. The net7.0-ios version of the bundle contains the 7.0 version of the assembly.

Version 8.0 of System.Net.Http.Json added new method overloads, and it still ships net7.0 builds in the NuGet package (alongside net6 and net8).

If someone updates the Nuget package to version 8.0 and builds for Hot Restart with net7-ios target it will result in “method not found” exception at runtime.

We update the packages through Dependabot and the new overloads actually map to existing usage (previously a different overload with default parameter would be used).

I assume to fix is to update Hot Restart bundle to either not ship outbox assemblies or reference their latest version.

Steps to Reproduce

1. dotnet new ios
2. Change TFM to net7.0-ios, add <PackageReference Include="System.Net.Http.Json" Version="8.0.0" />
3. Add this code to beginning of Program.cs:

   var httpContent = new ByteArrayContent("{\"Protocol\":\"Rest\",\"Url\":\"https://outlook.office.com/api\"}"u8.ToArray());
   await httpContent.ReadFromJsonAsync<AutodiscoverJsonResponse>(CancellationToken.None);

Expected Behavior

The app will start.

Actual Behavior

The app will immediately crash with

Method not found: System.Threading.Tasks.Task`1<!!0> System.Net.Http.Json.HttpContentJsonExtensions.ReadFromJsonAsync<!0>(System.Net.Http.HttpContent,System.Threading.CancellationToken)

The Hot Restart bundle overrides the NuGet version of System.Net.Http.Json.dll with the older 7.0.x version. This causes this particular overload to be missing.

---

DevDiv issue: https://devdiv.visualstudio.com/DevDiv/_queries/edit/1929615

[ivanpovazan]

/cc: @lambdageek

[filipnavara]

/cc: @lambdageek

Just to be clear. This is not a runtime issue.

It's the way the Hot Restart bundle is created. It comes with a pre-built app bundle that is augmented with the user application's assemblies. Since the pre-built bundle contains old version of System.Net.Http.Json.dll it gets loaded before the new one. It should either not be there in a first place (since it's an outbox assembly that has different distribution requirements from the inbox ones) or it needs to be regularly updated in the Hot Restart build process and released with the workloads.

/cc @mauroa @emaf

[rolfbjarne]

So it seems we bundle all the assemblies in the runtime packs:

$ ls -la /usr/local/share/dotnet/packs/Microsoft.NETCore.App.Runtime.Mono.ios-arm64/8.0.0/runtimes/ios-arm64/lib/net8.0/*.dll
-rwxr--r--@ 1 root  wheel   309016 Oct 31 15:58 /usr/local/share/dotnet/packs/Microsoft.NETCore.App.Runtime.Mono.ios-arm64/8.0.0/runtimes/ios-arm64/lib/net8.0/Microsoft.CSharp.dll
-rwxr--r--@ 1 root  wheel   429328 Oct 31 15:58 /usr/local/share/dotnet/packs/Microsoft.NETCore.App.Runtime.Mono.ios-arm64/8.0.0/runtimes/ios-arm64/lib/net8.0/Microsoft.VisualBasic.Core.dll
-rwxr--r--@ 1 root  wheel    17672 Oct 31 16:01 /usr/local/share/dotnet/packs/Microsoft.NETCore.App.Runtime.Mono.ios-arm64/8.0.0/runtimes/ios-arm64/lib/net8.0/Microsoft.VisualBasic.dll
[... a bunch more ...]
-rwxr--r--@ 1 root  wheel    59664 Oct 31 16:03 /usr/local/share/dotnet/packs/Microsoft.NETCore.App.Runtime.Mono.ios-arm64/8.0.0/runtimes/ios-arm64/lib/net8.0/mscorlib.dll
-rwxr--r--@ 1 root  wheel   101144 Oct 31 16:03 /usr/local/share/dotnet/packs/Microsoft.NETCore.App.Runtime.Mono.ios-arm64/8.0.0/runtimes/ios-arm64/lib/net8.0/netstandard.dll

If we want to not ship non-inbox assemblies, the question becomes: how do we distinguish them from the other assemblies?

Looking at the binlog, it seems the only difference could be the PublicKeyToken of the assembly, but that seems like a rather weird way to distinguish assemblies. FWIW here's a list of assemblies with their PublicKeyToken, there are 4 different tokens: https://gist.github.com/rolfbjarne/8df72e979ff088c7022c29bddd43457a

Alternatively we could hardcode the non-inbox assemblies, but that doesn't seem like a good idea either.

[filipnavara]

@ViktorHofer @akoeplinger Any insights on what outbox assemblies are shipped in runtime packs and why?

[rolfbjarne]

I wonder if deleting the file immediately after it's extracted from the prebuilt app bundle could be a workaround:

<Target Name="_RemoveOutboxAssemblies" AfterTargets="_PrepareHotRestartAppBundle">
    <ItemGroup>
        <OutboxAssemblies Include="$(HotRestartAppBundlePath)\System.Net.Http.Json" />
    </ItemGroup>
    <Delete Files="@(OutboxAssemblies)" />
</Target>

[filipnavara]

There are multiple workarounds. The one you mention is an option. We opted to downgrade the NuGet package for now since we don't need the new features. The new overloads are mostly a convenience (https://github.com/dotnet/runtime/issues/72103) and not really a feature or a bug fix. Unfortunately they get used with no source code changes in our case, and we don't have automated tests for Hot Restart builds to catch this error.

[akoeplinger]

@ViktorHofer @akoeplinger Any insights on what outbox assemblies are shipped in runtime packs and why?

It's rather the other way round: some in-box assemblies are shipped separately as out-of-box assemblies on nuget.org so people on older TFMs can use new features. This is mostly done for assemblies where there's a clear benefit.

In that case the OOB assembly should override the in-box assembly during the build in ResolveAssemblyReferences (because it has a higher version*).

I think we'll need to replicate that behavior somehow for Hot Restart. I don't know how exactly a Hot Restart build works, do we run a normal self-contained publish?

* note that the assembly version stays fixed at e.g. 7.0.0.0 but the assembly file version is something like 7.0.723.27404

[filipnavara]

I don't know how exactly a Hot Restart build works, do we run a normal self-contained publish?

It works by shipping a prebuilt iOS app w/ UseInterpreter=true and then augmenting it with user code. The user code basically comes from publish but the augmentation step is separate part of the iOS.Windows SDK workload (https://github.com/xamarin/xamarin-macios/blob/main/msbuild/Xamarin.iOS.Tasks.Windows/Xamarin.iOS.HotRestart.targets). IIRC the user assemblies are stored in a separate directory and don't override the ones coming from the prebuilt app, but I am not familiar with all the details of the ComputeHotRestartBundleContents task.

Doing version resolution seems like a reasonable thing to do. I assumed that the reason to include all framework assemblies was to AOT them and get better performance, but I checked and that's not done anyway.

[akoeplinger]

One thing we need to be careful about though is the runtime in the hot restart bundle won't get updated so e.g. we can't use a 7.0.5 System.Private.CoreLib.dll on a 7.0.0 runtime (well it probably works if the SPC<->runtime interface didn't change).

[rolfbjarne]

CC @emaf @mauroa

[emaf]

One thing we need to be careful about though is the runtime in the hot restart bundle won't get updated so e.g. we can't use a 7.0.5 System.Private.CoreLib.dll on a 7.0.0 runtime (well it probably works if the SPC<->runtime interface didn't change).

This is a great point. Is there a way to know what's safe to replace from the targets?

I don't like adding extra steps to make Hot Restart work but I wonder if this should be explicitly declared on the project file so folks don't end up with random runtime failures because assembly references are being replaced inadvertely.

[rolfbjarne]

@emaf mirror issue: https://devdiv.visualstudio.com/DevDiv/_queries/edit/1929615