dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.21k stars 9.95k forks source link

SignalR client: RPCs are not invoked on Android client if LLVM is on #33493

Open Tommigun1980 opened 3 years ago

Tommigun1980 commented 3 years ago

Describe the bug

My project is a Xamarin.Forms iOS/Android app. My project's shared project is a normal platform independent Xamarin.Forms .NET Standard 2.1 project. Said project is referencing the Microsoft.AspNetCore.SignalR.Client library.

There are several RPC registered to a HubConnection:

connection.On("SomeRPC", () =>
{
    Console.WriteLine("HELLO!");
});

These work flawlessly in all permutations of release configurations except one.

iOS: AOT + LLVM: Works -- RPCs are invoked (i.e. connection.On callbacks are invoked). Android: AOT: Works -- RPCs are invoked (i.e. connection.On callbacks are invoked). Android AOT + LLVM: Doesn't work! RPCs are never invoked.

To Reproduce

1) Set up a Xamarin.Forms application. 2) Add the Microsoft.AspNetCore.SignalR.Client NuGet to the shared project. Note: Use 6.0.0-preview1 because of this issue: https://github.com/dotnet/aspnetcore/issues/33269 3) Register some connection.On RPCs. 4) Use some means to invoke the RPCs (from some test server I guess). 5) Optional: Build and run a release build for iOS with AOT + LLVM on to verify that the RPCs are invoked. 6) Optional: Build and run a release build for Android with AOT on to verify that the RPCs are invoked. 7) Build and run a release build for Android with AOT + LLVM on to verify that the RPCs are not invoked.

(In the off-chance that Android LLVM builds fail to build, follow the workaround at https://github.com/xamarin/xamarin-android/issues/5764#issuecomment-856062396 until https://github.com/xamarin/xamarin-android/issues/5764 is fixed.)

So all in all, turning LLVM on for Android builds makes RPCs not being called anymore. This effectively prevents one from using LLVM on Android until this issue is fixed.

Exceptions (if any)

None. Breakpoints show that the RPCs are not invoked on Android when LLVM is turned on, but there are no exceptions.

Further technical details

Tommigun1980 commented 3 years ago

It's quite a pain to use these systems atm so I'll in case anyone else reads this and wants to actually make Xamarin builds for Android here are the hacks needed right now:

1) LLVM can't be turned on on Android unless AndroidAddKeepAlives is forced to false, until https://github.com/xamarin/xamarin-android/issues/5764 is fixed, unless you get lucky and the libraries you use are not subject to the https://github.com/xamarin/xamarin-android/issues/5764 issue. 2) SignalR client library can't be updated beyond 6.0.0-preview1 until https://github.com/dotnet/aspnetcore/issues/33269 is fixed, if you want to enable SDK linking on Android. 3) LLVM can't be turned on on Android until https://github.com/dotnet/aspnetcore/issues/33493 is fixed, as LLVM prevents RPCs from being called.

Hopefully this saves someone some time, but I'd really appreciate if Microsoft made sure that release builds work properly in all permutations. This is incredibly time consuming and it gets taxing to keep tons of lists of issues that need to be resolved before certain options can be turned on.

If you were a new user and wanted to compile a sample app, chances are very good that you'd never be able to do it. This pushes lots of potential users away so some more care would be greatly appreciated from all teams.

Thank you.

Tommigun1980 commented 3 years ago

Ping @davidfowl and @spouliot because you are active in the SignalR client library linker issue ticket, in case there's overlap between these issues. Thank you.

davidfowl commented 3 years ago

Most of .NET doesn't work with linkers until .NET 6. The SignalR client is no different in this regard. This hasn't been a priority in the past so not much works but it will going forward (probably .NET 7 and onward). Until then, you'll need to stop running the linker on libraries that don't explicitly support it or figure out which types you need to keep in order to make the SignalR client work.

Tommigun1980 commented 3 years ago

@davidfowl Thank you for the reply.

But this issue has nothing to do with linkers. I am saying that the SignalR client library doesn’t work with the LLVM compiler on Android. It does work with LLVM on iOS. When LLVM is enabled on Android RPC callbacks are not being invoked for some reason.

What do you mean with libraries not supporting the linker btw? All libraries I have ever used can be linked, except the past three preview versions of SignalR. When the linker is enabled it links all SDKs and the linker kinda has to be enabled unless you want your app to be impossibly large, it’s not run for certain libraries only. If a library can’t be linked it has some problem that needs to be fixed. There’s no way linker support could not be a priority for vendors. Maybe you are thinking of full program linking, but I am not using it, only SDK linking. Please remember that the SignalR client library is used in Xamarin mobile phone apps as well.

But that said, this ticket is not about linkers, that’s in the other ticket. This ticket is about the LLVM problem on Android.

https://docs.microsoft.com/en-us/xamarin/android/deploy-test/release-prep describes the LLVM under the section “LLVM Optimizing Compiler”.

SignalR is recommended for real-time communication in Xamarin apps by Microsoft: https://azure.microsoft.com/en-us/resources/videos/build-2019-crafting-real-time-mobile-apps-with-xamarin-and-signalr/

But apps using SignalR must be able to utilize LLVM if we are to release them to the public. It’s almost certainly a very minor detail that needs to be tweaked as LLVM should work out of the box. I’m sure it won’t take you guys many minutes to fix whatever issue there is.

If SignalR is no longer supposed to work in mobile apps then that has not been documented. But for mobile apps to be able to be released, we have to make optimized builds before submitting the apps to the app stores and to customers. Libraries must be mindful of the different environments where they can be used, especially if they are the recommended practices by Microsoft itself. It is extremely time consuming to figure out which library doesn’t play by the rules, especially when you are following all the best practices that Microsoft has laid out. In its most banal form, a lot of sample apps can’t be compiled right now as it stands.

Thank you.

davidfowl commented 3 years ago

I'm just guessing that these AOT compiler also do linking (that's what they typically do) and if so, it breaks libraries because things get removed that shouldn't. I'm 80% sure this is the issue, if there's something else wrong then we'd be happy to fix but as I said before, we don't test our libraries in these environments and until .NET 7 we don't be investing in a linker story outside of what we're planning to do now.

If a library can’t be linked it has some problem that needs to be fixed

Right, see my previous comment. Making libraries linker friendly has been focus for .NET 6 with Blazor, Xamarin will converge on that same plan in .NET 6. Before then, you just got lucky. Most of the libraries we ship, including SignalR don't work with the linker and I don't expect the mass of libraries to work that are on NuGet.

I’m sure it won’t take you guys many minutes to fix whatever issue there is.

We don't plan to get to this issue anytime soon, at least during the .NET 6 timeframe. If it's something that can be easily debugged and you can pinpoint something easy we can fix, we'll take it, beyond that, we're not going to take any action here.

davidfowl commented 3 years ago

If SignalR is no longer supposed to work in mobile apps then that has not been documented

I can get behind this. We should open a doc issue explaining that the SignalR client isn't AOT compatible on Xamarin. We'll turn this into a documentation issue until we have the proper support in the product.

Tommigun1980 commented 3 years ago

Thank you for the replies @davidfowl. But this issue still isn’t related to linkers.

As I said in the ticket the SignalR library does work with AOT on Android. It also works with AOT + LLVM on iOS. What does not work, however, is AOT + LLVM on Android.

I am asking to please fix the LLVM bug on Android. It is pretty much mandatory for apps using SignalR that are to be released on Google Play.

Thank you again.

Tommigun1980 commented 3 years ago

I can get behind this. We should open a doc issue explaining that the SignalR client isn't AOT compatible on Xamarin. We'll turn this into a documentation issue until we have the proper support in the product.

Wait... If you are turning any Xamarin issues into documentation issues you are effectively deprecating support for Xamarin.

I have spent a year making an app and now you are telling me Xamarin issues will be turned into documentation issues until .NET 7?

Is this an official stance? That is literally killing off Xamarin support, as we won’t be able to really make releases for a few years then.

Tommigun1980 commented 3 years ago

I honestly think that you guys should take half a day into fixing any linker issues, and then fix whatever is preventing LLVM from working on Android. These are most of the time trivial issues. Do you really want to deprecate SignalR as the framework for Microsoft’s mobile strategy, over something that would be very much fixable in a very short time?

davidfowl commented 3 years ago

SignalR doesn't support linking in any form. We want to, we just have never done so. I've mentioned this several times. This includes AOT compiled Xamarin platforms that perform or Unity without work on the user's end part. Many people have gotten SignalR to work on these platforms but

We'll gladly fix issues identified by passionate members such as yourself who report them and help us support the existing platforms. Beyond that, we have no plans to do anything here in the short term.

davidfowl commented 3 years ago

Consider using websockets directly if SignalR doesn't meet your needs. We'll be looking into these problems in a more concerted effort in a future release beyond the existing bug reports we get today.

I honestly think that you guys should take half a day into fixing any linker issues

We'll definitely do that time permitting, no promises as to when that day will occur!

Tommigun1980 commented 3 years ago

Consider using websockets directly if SignalR doesn't meet your needs

It's not about "SignalR" not meeting my needs, it's about it correctly working on all of the platforms it's supposed to support. If it supports Xamarin (which it should as there's lots of Microsoft materials about integrating SignalR to Xamarin apps), then by extension Xamarin.Android should be supported. But if you can't build optimised LLVM builds then it isn't really supported. It's really not about SignalR not meeting my needs as I said.

Also it's too late to change technologies anymore as the product is almost done, nor would I want to change up everything for a minor issue like LLVM not working properly on Android. (Where minor refers to cost of fixing the issue on your end, even though the issue itself is large to us developers who use SignalR in mobile apps as it prevents them from being released).

ghost commented 3 years ago

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.