Open Tommigun1980 opened 3 years ago
Edit: Updated report to reflect that this is indeed an ASP.NET Core issue and not a Mono/Xamarin-macios issue.
Looks like it might be due to a the linker removing this constructor somehow.
cc @eerhardt
Looking. My thinking is that the linker used by Xamarin Forms is older and doesn't understand DynamicallyAccessedMembersAttribute
. But if that is true, then I'm not sure how it worked before 6.0.0-preview2.
Looking. My thinking is that the linker used by Xamarin Forms is older and doesn't understand
DynamicallyAccessedMembersAttribute
. But if that is true, then I'm not sure how it worked before 6.0.0-preview2.
I am unsure exactly in which version it broke (preview2, 3, or 4). preview-1 works properly. I did test with 2, 3 and 4 but failed to delete obj and bin folders so my testing was polluted. I have no idea why deleting them has any bearing as it is normally not a requirement when upgrading/downgrading NuGet packages, but clearly some kind of state was left behind when jumping between versions in this case. So I am content on preview-1 for now but am locked out from any new updates until this is fixed.
Thank you.
UnnamedOptionsManager
is a new type in a recent preview of 6.0, could that be related to the issue.
@eerhardt who can we ping on the Xamarin side to figure this out?
Could you try out customizing the linker settings to see if it works around the issue? (adding the UnnamedOptionsManager
type or full namespace)
https://docs.microsoft.com/en-us/xamarin/cross-platform/deploy-test/linker
@spouliot @rolfbjarne - any thoughts here? Does Xamarin's linker understand DynamicallyAccessedMembers
?
@eerhardt in net6 there's no Xamarin's linker anymore. It's the net6 linker and a few additional (no removal) steps. IOW it it works in net6 then it works with Xamarin.iOS.
If this is about legacy then I don't think it's supported. The linker is based on mono 2020-02, only with more (and different) customization.
My understanding from the description of the issue is that this is using the legacy Xamarin Forms with the 6.0 version of Microsoft.Extensions.Options.
No I am not using legacy Xamarin Forms. I'm using Xamarin.Forms 5, the current release that will be supported 'til November 2022. If Microsoft is working on a new version that may be released in the coming year or so, I wouldn't know about it.
So the current version of Xamarin.Forms with the latest NuGet packages of everything.
Here's what we can do to clear things up:
As for the current release of Xamarin.Forms, I don't know how it worked before or why it worked out of the box. I assume something in your code (or in a hidden linker file or extensibility point) was rooting these types. It's nothing that was influenced by SignalR itself.
UnnamedOptionsManager is a new internal type that was introduced in .NET 6 preview4 but it's an implementation detail. The old type used in its place was OptionsManager. It's possible that OptionsManager was rooted by something else (we need to figure out what that was) and that UnnamedOptionsManager isn't rooted.
This is the crux of the issue.
As for which linker tech is being used, it seems like we're talking about the linker before it understood these annotations.
Here's what we can do to clear things up:
- SignalR hasn't done any work to make it work well with a linker in any scenario. If it worked before you got lucky, or something else was rooting code that made it work. This is similar to unity workloads that break left and right with IL linkers.
- .NET 5 introduced a new way for libraries to annotate their code to influence the linker and we've used this in a couple of places in .NET and ASP.NET Core to make it more linker friendly.
- .NET 6 does much more of this annotating and fixing things so that we can be more confident running the linker will work or will warn when its going to break.
As for the current release of Xamarin.Forms, I don't know how it worked before or why it worked out of the box. I assume something in your code (or in a hidden linker file or extensibility point) was rooting these types. It's nothing that was influenced by SignalR itself.
UnnamedOptionsManager is a new internal type that was introduced in .NET 6 preview4 but it's an implementation detail. The old type used in its place was OptionsManager. It's possible that OptionsManager was rooted by something else (we need to figure out what that was) and that UnnamedOptionsManager isn't rooted.
This is the crux of the issue.
As for which linker tech is being used, it seems like we're talking about the linker before it understood these annotations.
Thank you so much for the very thorough explanation, I really appreciate it!
Is there anything I can do to make the linker keep the relevant parts, maybe by referencing something explicitly to keep it included or such?
Or will XF5 compatibility be added to the SignalR library itself through some means?
Thanks for looking into this!
Is there anything I can do to make the linker keep the relevant parts, maybe by referencing something explicitly to keep it included or such?
Yes
try out customizing the linker settings to see if it works around the issue? (adding the UnnamedOptionsManager type or full namespace) https://docs.microsoft.com/en-us/xamarin/cross-platform/deploy-test/linker
Or, you could try out the latest 6.0 version of Xamarin and see if the new linker works.
@spouliot for the old linker, were there hardcoded classes that the linker didn't remove?
@spouliot for the old linker, were there hardcoded classes that the linker didn't remove?
@BrennanConroy Not really. At least not for supporting code that is not shipped part of our SDK (including SignalR).
IOW in the current (soon legacy) SDK we add linker support for the BCL we ship (as part of the SDK). That can be achieve by different forms, but that's transparent to developers.
The default linker mode for app is "Link SDK" which means the linker will only remove code from assemblies that we ship.
In order for the linker to remove code in "user code" (assemblies not part of the SDK) then the linker mode must be set to "Link all". That also means the developer becomes responsible of ensuring the code is linker safe, e.g. preserving code accessed thru reflection.
Turn on linking ("Link SDKs only")
That does not seems to be the case here... and it would mean UnnamedOptionsManager
is not removed.
What might be happening is that the app has code (likely an assembly binary) compiled against "Assembly 6.0" (that would include references to UnnamedOptionsManager
) but builds the app against Assembly 5.0
(that would not have UnnamedOptionsManager
).
That can work for a JIT app, since such errors will happen at runtime, and you might never hit the code path that would throw an exception.
However that often won't work for AOT (required for iOS) since the error will happen at build time [1]. Same thing for the linker... it won't be able to resolve the metadata and fail [1]
[1] unless the code is only used thru reflection... in that case the error will also be at runtime
@spouliot Correct, I am using the default linking option of "Link SDK assemblies only", where the responsibility for the preservation of SDK constructs should not be brought over to me/the app developer.
Here is a list of libraries that the client's shared (non-native) project is referencing:
<PackageReference Include="Xamarin.Forms" Version="5.0.0.2012" />
<PackageReference Include="Xamarin.Essentials" Version="1.6.1" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
<PackageReference Include="Xamarin.FFImageLoading.Transformations" Version="2.4.11.982" />
<PackageReference Include="Xamarin.FFImageLoading.Svg.Forms" Version="2.4.11.982" />
<PackageReference Include="UXDivers.Grial" Version="3.2.83" />
<PackageReference Include="Rg.Plugins.Popup" Version="2.0.0.11" />
<PackageReference Include="Xamarin.FFImageLoading.Forms" Version="2.4.11.982" />
<PackageReference Include="XamarinUniversity.Infrastructure" Version="2.2.0" />
<PackageReference Include="ServiceConcurrency" Version="1.1.6" />
<PackageReference Include="LightObjectMapper" Version="1.0.6" />
<PackageReference Include="LightHttpRequest" Version="1.0.5" />
<PackageReference Include="Plugin.FacebookClient" Version="3.0.7-beta" />
<PackageReference Include="Xamarin.Forms.MultiDependencyResolver" Version="1.0.0" />
<PackageReference Include="Microsoft.AppCenter.Crashes" Version="4.2.0" />
<PackageReference Include="IOSToolbarExtensions" Version="1.0.0" />
<PackageReference Include="Xamarin.Forms.Chips" Version="1.0.13" />
<PackageReference Include="AnalyticsSender.Sinks.AppCenterAnalyticsSink" Version="1.0.0" />
<!-- Don't update these until https://github.com/dotnet/aspnetcore/issues/33269 is fixed. Last tested version: 6.0.0-preview4. -->
<PackageReference Include="Microsoft.AspNetCore.SignalR.Client" Version="6.0.0-preview.1.21103.6" />
<PackageReference Include="Microsoft.Extensions.Logging.Console" Version="6.0.0-preview.1.21102.12" />
<!-- Don't update this until https://github.com/xamarin/XamarinCommunityToolkit/issues/1209 is merged. Last tested version: 1.2.0-pre2. -->
<PackageReference Include="Xamarin.CommunityToolkit" Version="1.1.0-preview1056" />
The problem went away when I downgraded the SignalR client library back to 6.0.0-preview1. While I was jumping between different SignalR client library preview versions to isolate where it broke, I had to restart VS and then delete bin and obj folders in the project for the changes to take effect; merely changing the package versions and restoring packages wasn't enough for some reason.
The project where the SignalR client library resides in is as mentioned the 'shared' platform-independent portion in a Xamarin.Forms app (ie no iOS or Android specific code in there). It is using the default .NET target framework of ".NET Standard 2.1". My Visual Studio for Mac version is 8.10 and Xamarin.Forms version is 5.0.0.2012 (in case it has any bearing on this).
Please let me know if you need any other clarifications. Thank you!
@spouliot which repo should we move this issue to?
UnnamedOptionsManager is a new internal type that was introduced in .NET 6 preview4
@davidfowl that's not P4 of SignalR - but P4 for .net6 right ?
It sounds like the type is not removed (by the linker) but a type that only exists in net6.
That would makes SignalR 6.0 incompatible with netstandard 2.1, which is what the customer is using.
Is SignalR nuget advertising support for anything before net6 ?
a type that only exists in net6.
It is a type in the Microsoft.Extensions.Options package, which targets netstandard2.0, so it is compatible with netstandard2.1.
Is SignalR nuget advertising support for anything before net6 ?
Yes, we target net461, netstandard2.0, and net6.0
UnnamedOptionsManager is a new internal type that was introduced in .NET 6 preview4
@davidfowl that's not P4 of SignalR - but P4 for .net6 right ?
It sounds like the type is not removed (by the linker) but a type that only exists in net6.
That would makes SignalR 6.0 incompatible with netstandard 2.1, which is what the customer is using.
Is SignalR nuget advertising support for anything before net6 ?
Hi. The type is very much removed by the linker, as when linking is disabled everything works. If that type didn't exist I wouldn't even be able to compile my app. But yeah, everything works with 6.0.0-preview4 if the linker is completely disabled, or when downgrading to 6.0.0-preview1. Also, if only .NET6 is supported then you are killing of Xamarin.Forms support etc. Documentation doesn't mention this support being removed.
Thank you.
It is a type in the Microsoft.Extensions.Options package
@BrennanConroy hmm... ok, but that just move the target/questions elsewhere as this package is not directly referenced by the customer's project, so we don't know which "working" version "p1" used versus the "broken" version "p2" is using...
and that's likely true for many other dependencies used in the project: bump one and you get a cascade of updates.
which repo should we move this issue to?
If it worked in 6.0.0-preview1 and broke in 6.0.0-preview2 then this issue is already in the right repo, at least for the moment.
@Tommigun1980 thanks for the information you have provided. However it is, sadly, of little use to diagnose the issue. There's something weird as the linker would not remove a "user" (non SDK) type. In most cases this type of issues involve assemblies binaries that were build against a different version of the dependencies (than the one your app is providing).
There's not much, beside guessing, that can be done without a test case to reproduce the issue. Are you able to attach a test case (ideally a subset of what you have) we can look into ? If you can provide this then I'll have a look, because I'm curious :-)
If not then another (less optimal) way to find what happened is to the build your project doing binary search with different builds, between p1 and p2, to find which commit cause the issue. It can be done with sources, but it's easier if you have binaries.
@BrennanConroy does your CI publish every build somewhere @Tommigun1980 can access ?
Before doing a binary search I recommend coming up with the most minimal repro using Dependency injection with options in a Xamarin application
@spouliot Xamarin.CommunityToolkit also had problems with the linker removing the default constructor for a type (SafeAreaEffectRouter) and they fixed it like so: https://github.com/xamarin/XamarinCommunityToolkit/pull/455/commits/7ef212887266700312095922c802ecd1d2a7357e
It was the exact same problem that builds with the linker enabled ("Link SDKs only") would remove the default constructor, and they fixed it in the above commit. Maybe @pictos could chime in with details?
@Tommigun1980 no, XCT explicitly told the linker it was safe to process their assembly.
By doing so they get the benefits (smaller size, optimizations) but , like spidey said, with great power comes great responsibility :-)
Before doing a binary search
@davidfowl definitively! A test case was my first suggestion :-)
@spouliot I made the bug report for XCT. One of their releases was broken when "Link SDKs" was enabled, exactly the same case as is happening here. The linked commit was the fix to my bug report. Once it landed "Link SDKs" was working again. Before that commit the error was almost identical to the one encountered here, with a missing default constructor.
The [assembly: LinkerSafe]
was added a month after the linker issue was introduced and fixed. At least https://github.com/xamarin/XamarinCommunityToolkit/commit/7ef212887266700312095922c802ecd1d2a7357e was marked as the commit that fixed the issue I reported where "Links SDKs" got broken.
The [assembly: LinkerSafe] was added a month after the linker issue was introduced and fixed.
@Tommigun1980 no, the attribute was added earlier, on August 20th [1], while you filed the issue on October 8th 2020 [2]
[1] https://github.com/xamarin/XamarinCommunityToolkit/pull/210/files [2] https://github.com/xamarin/XamarinCommunityToolkit/issues/393
But that does not help finding your current issue. If the linker removes the .ctor then something told it to process that assembly. I've been told no for project options and for the assemblies opt'ing-in.
It was the exact same problem that builds with the linker enabled ("Link SDKs only") would remove the default constructor, and they fixed it in the above commit. Maybe @pictos could chime in with details?
I believe that @spouliot is aware of that workaround, I learned it from him here (; I'm reading the role thread to have context, but looks like an interesting issue... AFAIK net6 doesn't the Mono's linker (Xamarin.Forms needs mono to build) and mono doesn't know .NET6's linker, so that could be an issue? (I'm guessing here)
We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.
Ok, I finally got my mac all updated and working so I could test this out and I can now say I understand why this is happening.
Between 6.0-preview1 and 6.0-preview2, we added the [AssemblyMetadata("IsTrimmable", "true"]
attributes to all the assemblies shipping from dotnet/runtime in https://github.com/dotnet/runtime/pull/48428.
It appears that the Xamarin iOS mtouch tool, which contains a version of the ILLinker code, is now starting to respect that assembly attribute as a way of telling if the assembly should be trimmed or not. See https://github.com/xamarin/xamarin-macios/pull/11229.
If I look at the mtouch.exe
on my machine in ILSpy I see the following code:
protected override bool IsLinkerSafeAttribute(CustomAttribute attribute)
{
TypeReference attributeType = attribute.get_AttributeType();
string name = ((MemberReference)attributeType).get_Name();
string text = name;
if (!(text == "LinkerSafeAttribute"))
{
if (text == "AssemblyMetadataAttribute")
{
if (!attribute.get_HasConstructorArguments())
{
return false;
}
if (attributeType.get_Namespace() != "System.Reflection")
{
return false;
}
CustomAttributeArgument val = attribute.get_ConstructorArguments().get_Item(0);
if (((CustomAttributeArgument)(ref val)).get_Value() as string!= "IsTrimmable")
{
return false;
}
val = attribute.get_ConstructorArguments().get_Item(1);
return ((CustomAttributeArgument)(ref val)).get_Value().ToString().ToLowerInvariant() == "true";
}
return false;
}
return true;
}
This logic is used to tell whether to set the AssemblyAction.Link
action on the assembly or not.
However, the issue is, the iOS mtouch tool should only be respecting the IsTrimmable
assembly attribute if it also respects all the other attributes we've added to the new version of the linker: DynamicallyAccessedMembers
, DynamicDependency
, etc. Just because an assembly has IsTrimmable
on it, doesn't mean that it can be trimmed without also respecting these new attributes.
I believe this issue should be moved to https://github.com/xamarin/xamarin-macios, as the current version of the mtouch tool is not doing the right thing.
cc @spouliot @sbomer
@eerhardt thanks for your investigation! That was meant to help/ease code move to net6, not to downgrade net6 code back to netstandard.
Between 6.0-preview1 and 6.0-preview2, we added the [AssemblyMetadata("IsTrimmable", "true"] attributes to all the assemblies shipping from dotnet/runtime in dotnet/runtime#48428.
That remains a problem if some of the dotnet/runtime assemblies do not plan to support being trimmable for net6.
That remains a problem if some of the dotnet/runtime assemblies do not plan to support being trimmable for net6.
We have backtracked on a few assemblies that we don't plan on supporting trimmability. See https://github.com/dotnet/runtime/pull/52272 and https://github.com/dotnet/runtime/pull/49085. The current list of libraries that don't plan on supporting trimming in the near future:
For those assemblies we don't add the IsTrimmable
assembly metadata - https://github.com/dotnet/runtime/search?q=SetIsTrimmable
But the rest of the assemblies we ship in the .NET 6 timeframe will have IsTrimmable=true
on them. So anyone using our v6 OOBs on the current Xamarin stack will run into this issue.
Thank you guys so much for looking into this!! I really appreciate it.
@spouliot I am assuming that commit fixes linking Xamarin apps when using the new .NET 6 libraries, right? If so do you know when it will be released? Iirc there was a new VS For Mac update a couple of days ago with a new Xamarin.iOS version, did the fix make it to that version?
Thank you again everyone!
I am getting this when I try to connect.. Type Microsoft.AspNetCore.Http.Connections.Client.HttpConnection has invalid vtable method slot 16 with method non e
Just for googlability, Im also using xamarin, and "link all", so Im also stuck on signalr v5.
Tried 6.0.1 and 6.0.10 and both give me this message(on iOS only):
The type initializer for 'Microsoft.AspNetCore.Http.Connections.Client.Internal.Constants' threw an exception.
at Microsoft.AspNetCore.Http.Connections.Client.Internal.Constants.GetUserAgentHeader () [0x0003f] in <0ff77aac3a5348fa82d58cb6c49bd84c>:0 at Microsoft.AspNetCore.Http.Connections.Client.Internal.Constants..cctor () [0x00018] in <0ff77aac3a5348fa82d58cb6c49bd84c>:0
"link sdk only" works fine. Also tried making a linker exception(preserve all) for "Microsoft.AspNetCore.Http.Connections.Client", but does not help...
Describe the bug
When Microsoft.AspNetCore.SignalR.Client is updated from 6.0.0-preview1 to preview4 (and possibly preview2 and 3) it no longer works in a Xamarin.Forms (iOS) project if the linker is enabled.
6.6.0-preview1 and earlier versions work fine even when the linker is enabled.
To Reproduce
System.InvalidOperationException: A suitable constructor for type 'Microsoft.Extensions.Options.UnnamedOptionsManager'1[Microsoft.AspNetCore.Http.Connections.Client.HttpConnectionOptions]' could not be located. Ensure the type is concrete and services are registered for all parameters of a public constructor.
".The crash only happens if linking is enabled and the version is 6.0.0-preview4 (I verified that preview1 works, am unsure exactly where it got broken). The full stack trace is as follows:
The code it complains about, HubConnectionBuilder.Build, will crash even with an empty connection builder:
Please note that this is a Xamarin.Forms project, in case it has any bearing on anything.
It goes without saying that this is a catastrophic failure and I'd really appreciate a fix. Thank you.
Exceptions (if any)
Further technical details