Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NSUrlSession implementation of HttpClient throws TaskCanceledException immediately after backgrounding #6443

Closed
alanag13 opened this issue Jun 27, 2019 · 28 comments · Fixed by #6918
Assignees
Labels
bug If an issue is a bug or a pull request a bug fix iOS Issues affecting Xamarin.iOS
Milestone

Comments

@alanag13
Copy link

When using the NSUrlSession implementation of HttpClient, backgrounding the app while a request is being made results in TaskCancelationException immediately being thrown.

Steps to Reproduce

  1. Open the attached reproduction project and run in an iOS simulator (mine was iOS 12.2, but Im guessing other versions are affected)
  2. Launch the application.
  3. Within 10 seconds of the first screen appearing, background the application.
  4. Change the project settings to use the Managed HttpClient instead of NSUrlSession.
  5. Clean and rebuild the iOS project.
  6. Repeat the above.

Expected Behavior

TaskCanceledException should not immediately be thrown for either the NSUrlSession or Managed implementation of HttpClient

Actual Behavior

TaskCanceledException is immediately thrown when using the NSUrlSession implementation of HttpClient

Environment

Visual Studio Community 2019 for Mac
Version 8.1.2 (build 2)
Installation UUID: eb26d0fb-e6cf-4f52-b105-f2a0af707e44
	GTK+ 2.24.23 (Raleigh theme)
	Xamarin.Mac 5.6.0.25 (d16-0 / 50f75273)

	Package version: 518010028

Mono Framework MDK
Runtime:
	Mono 5.18.1.28 (2018-08/223ea7ef92e) (64-bit)
	Package version: 518010028

Xamarin Designer
Version: 16.1.0.467
Hash: f1657e133
Branch: remotes/origin/d16-1-new-document-model
Build date: 2019-06-18 21:57:42 UTC

NuGet
Version: 5.0.2.5988

.NET Core
Runtime: /usr/local/share/dotnet/dotnet
Runtime Versions:
	2.1.11
	2.1.9
	2.1.2
	2.0.5
SDK: /usr/local/share/dotnet/sdk/2.1.700/Sdks
SDK Versions:
	2.1.700
	2.1.505
	2.1.302
	2.1.4
MSBuild SDKs: /Library/Frameworks/Mono.framework/Versions/5.18.1/lib/mono/msbuild/Current/bin/Sdks

Xamarin.Profiler
Version: 1.6.10
Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler

Updater
Version: 11

Xamarin.Android
Version: 9.3.0.23 (Visual Studio Community)
Commit: HEAD/d0b48056f
Android SDK: /Users/alan/Library/Developer/Xamarin/android-sdk-macosx
	Supported Android versions:
		7.0 (API level 24)
		8.0 (API level 26)
		8.1 (API level 27)

SDK Tools Version: 26.1.1
SDK Platform Tools Version: 28.0.0
SDK Build Tools Version: 27.0.3

Build Information: 
Mono: mono/mono/2018-08@3a07bd426d3
Java.Interop: xamarin/java.interop/d16-1@5ddc3e3
LibZipSharp: grendello/LibZipSharp/d16-1@44de300
LibZip: nih-at/libzip/rel-1-5-1@b95cf3f
ProGuard: xamarin/proguard/master@905836d
SQLite: xamarin/sqlite/3.27.1@8212a2d
Xamarin.Android Tools: xamarin/xamarin-android-tools/d16-1@acabd26

Microsoft Mobile OpenJDK
Java SDK: /Users/alan/Library/Developer/Xamarin/jdk/microsoft_dist_openjdk_1.8.0.25
1.8.0-25
Android Designer EPL code available here:
https://github.com/xamarin/AndroidDesigner.EPL

Android Device Manager
Version: 1.2.0.44
Hash: aac645b
Branch: remotes/origin/d16-1
Build date: 2019-05-29 19:55:24 UTC

Apple Developer Tools
Xcode 10.2.1 (14490.122)
Build 10E1001

Xamarin.Mac
Version: 5.10.0.157 (Visual Studio Community)
Hash: 6bd94753
Branch: d16-1
Build date: 2019-06-12 17:28:48-0400

Xamarin.iOS
Version: 12.10.0.157 (Visual Studio Community)
Hash: 6bd94753
Branch: d16-1
Build date: 2019-06-12 17:28:47-0400

Xamarin Inspector
Version: 1.4.3
Hash: db27525
Branch: 1.4-release
Build date: Mon, 09 Jul 2018 21:20:18 GMT
Client compatibility: 1

Build Information
Release ID: 801020002
Git revision: 5d9e30e11e16dbb655977e4f7051ffedd3de9f94
Build date: 2019-06-24 20:05:44+00
Build branch: release-8.1
Xamarin extensions: 5104b76623e0df8e5f763f87d170279c0b788e89

Operating System
Mac OS X 10.14.5
Darwin 18.6.0 Darwin Kernel Version 18.6.0
    Thu Apr 25 23:16:27 PDT 2019
    root:xnu-4903.261.4~2/RELEASE_X86_64 x86_64

Build Logs

buildlogs.txt

Example Project (If Possible)

TaskCanceledRepro.zip

@czuvich
Copy link

czuvich commented Jun 27, 2019

Ahh I bet there's a relationship between these two bugs.
#6387

@alanag13
Copy link
Author

alanag13 commented Jul 2, 2019

Can someone confirm this as a bug? @spouliot ?

@spouliot
Copy link
Contributor

spouliot commented Jul 3, 2019

stack trace

{System.Threading.Tasks.TaskCanceledException: A task was canceled.
  at System.Net.Http.NSUrlSessionHandler.SendAsync (System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) [0x001d4] in /Users/poupou/git/master/xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:432 
  at System.Net.Http.HttpClient.SendAsyncWorker (System.Net.Http.HttpRequestMessage request, System.Net.Http.HttpCompletionOption completionOption, System.Threading.CancellationToken cancellationToken) [0x0009e] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/mcs/class/System.Net.Http/System.Net.Http/HttpClient.cs:281 
  at TaskCanceledRepro.MainPage.OnAppearing () [0x0002a] in /Users/poupou/Downloads/TaskCanceledRepro/TaskCanceledRepro/MainPage.xaml.cs:26 
  at System.Runtime.CompilerServices.AsyncMethodBuilderCore+<>c.<ThrowAsync>b__7_0 (System.Object state) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/mcs/class/referencesource/mscorlib/system/runtime/compilerservices/AsyncMethodBuilder.cs:1021 
  at Foundation.NSAsyncSynchronizationContextDispatcher.Apply () [0x00000] in /Users/poupou/git/master/xamarin-macios/src/Foundation/NSAction.cs:178 }

@spouliot spouliot added iOS Issues affecting Xamarin.iOS bug If an issue is a bug or a pull request a bug fix labels Jul 3, 2019
@spouliot
Copy link
Contributor

spouliot commented Jul 3, 2019

There's no exception on the managed handler - but it is not aware of the OS state (like being in the background). Still I would have expected some delay...

@mandel-macaque any clue since you already looked at background support ?

@spouliot spouliot added this to the Future milestone Jul 3, 2019
@spouliot
Copy link
Contributor

spouliot commented Jul 3, 2019

The cancellation comes from

		void BackgroundNotificationCb (NSNotification obj)
		{
			// we do not need to call the lock, we call cancel on the source, that will trigger all the needed code to 
			// clean the resources and such
			foreach (var r in inflightRequests.Values) {
				r.CompletionSource.TrySetCanceled ();
			}
		}

so it's unrelated to #6387 - and looks to be by-design.

@alanag13
Copy link
Author

alanag13 commented Jul 3, 2019

Is this a recent change? I don't see this behavior in x.Android either, and I could be wrong of course but I feel like I could safely background in the past.

@marnilss
Copy link

marnilss commented Jul 8, 2019

We're also experiencing this (on an iOS device):

  1. Make call to UIApplication.SharedApplication.BeginBackgroundTask(timedOutHandler)
  2. Call code that eventually makes a request with HttpClient (using NsUrlSessionHandler)
  3. Press Home button on device
  4. A TaskCanceledException is thrown:
System.Threading.Tasks.TaskCanceledException: A task was canceled.
at System.Net.Http.NSUrlSessionHandler.SendAsync (System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) [0x001d4] in /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:342
at System.Net.Http.HttpClient.SendAsyncWorker (System.Net.Http.HttpRequestMessage request, System.Net.Http.HttpCompletionOption completionOption, System.Threading.CancellationToken cancellationToken) [0x00080] in /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/external/mono/mcs/class/System.Net.Http/System.Net.Http/HttpClient.cs:276

Hoping for a quick resolution to this.

Best regards,
Marcus Nilsson

@alanag13
Copy link
Author

alanag13 commented Jul 9, 2019

It looks like this behavior was introduced in this PR:
#5463

Based on the description text of that PR, it does look like this change was intentional -- I'm guessing the only way around this is to catch TaskCancelledException and retry.

@marnilss
Copy link

marnilss commented Jul 9, 2019

It looks like this behavior was introduced in this PR:
#5463
Based on the description text of that PR, it does look like this change was intentional -- I'm guessing the only way around this is to catch TaskCancelledException and retry.

But it then means that backgrounding tasks using UIApplication.SharedApplication.BeginBackgroundTask (https://robgibbens.com/backgrounding-with-xamarin-forms/) will not work anymore (broken by this change). It had the upside that you could actually continue requests using the HttpClient even when it went into background (started when in foreground). I'm aware of the limitations (limited amount of time available) but it was/is a nice way to have medium-long requests finalize in the background.

@mandel-macaque
Copy link
Member

mandel-macaque commented Jul 9, 2019

First, let me give some context on why that was done and why I think is the correct fix. The change was introduced in PR #5463 to fix issue #5333.

As the comment of that PR states, the mono threadpool gets into an undefined state when the application goes into the background. This means, that when you were using the async/await pattern in the application and you do not create the NSUrlSessionHandler using a background session, once the application is returned to the foreground will be blocked.
What we do now is to ensure that if your user does leave the application, that when she comes back, the app is not blocked.

You may be wondering, what benefit is this for you application? Now you know that the request did not complete, and that the app works and will not block. This probably has moved some users of your applications from experiencing freezes, to exceptions (that probably killed and relaunched the app).
From a user perspective (not a developer, but the user of your app), the application is more reliable and does not freeze. From the point of the developer this means that some extra work needs to be done. There are different ways to deal with this:

Continue using the HttpClient as you have configured

In this approach, you have to be aware that applications do go to the background and requests can be cancelled. You may consider this a regression, since it does happen ASAP. There is a reason why I wanted to do it ASAP. We do not control the time it takes the mono threadpool to do its cleanup, meaning that if we are not doing this as soon as we can, we might end up with a frozen app. If the requests performed by the application are short enough, the cancellation should not be a huge deal, and you can always use a try/catch to retry the request. Or even notify the user and retry.
Try/catch and retrying will provide a better user experience to your clients over frozen/unresponsive apps.

Using a NSUrlSessionHandler configured to work in the background

Creating a background session is a simple process and will ensure that, when the application is moved to the background, the request completes successfully (assuming no issues with the network etc..) and that the application will not freeze when it moves back to the foreground.

Creating a HttpClient that uses a background session handler is quite easy:

// create a background configuration for the application
var configuration = NSUrlSessionConfiguration.CreateBackgroundSessionConfiguration ("my.app.identifier");
// set any specific requirements in the configuration, in this case, we are downloading a lot of data, do always use wifi
configuration.AllowsCellularAccess = false;
// create the client using the configuration
var client = new HttpClient (new NSUrlSessionHandler (configuration));

Summary

With a HttpClient, you have to be aware that requests can be cancelled. You may consider this a regression, but we do not control the time it takes the mono threadpool to cleanup. If we do not start that as soon as possible, it is possible to end up with a frozen app. If the requests performed by the application are short enough, the cancellation should not be a problem, and you can use a try/catch to retry the request. Else, a background session configured NSUrlSessionHandler is recommended.

I will close this issue, since this was done by design. There is an option in which we could introduce an API to allow you to ignore the fact that the application goes to the background, but as mentioned above, this will possible freeze the application

@mandel-macaque
Copy link
Member

I this was not clear in the release notes, we should make sure that this does not happen again. Which I take full responsibility.

@alanag13
Copy link
Author

alanag13 commented Jul 9, 2019

@mandel-macaque thank you for the detailed response and explanation. I understand the motivation behind this change better now, but I agree that it should have been communicated more clearly. Thanks again for taking the time to respond.

@mandel-macaque
Copy link
Member

@alanag13 no problem. We are trying to improve our release notes to make sure that this does not happen again. It is an item we are actively working on.

@chamons
Copy link
Contributor

chamons commented Jul 9, 2019

Yeah, we have a number of tags we're using to try to improve our release notes (so we don't miss things like this again).

Sorry for the trouble.

@yaliashkevich
Copy link
Contributor

yaliashkevich commented Aug 8, 2019

@mandel-macaque,

As the comment of that PR states, the mono threadpool gets into an undefined state when the application goes into the background. This means, that when you were using the async/await pattern in the application and you do not create the NSUrlSessionHandler using a background session, once the application is returned to the foreground will be blocked.
What we do now is to ensure that if your user does leave the application, that when she comes back, the app is not blocked.

app still may be blocked due to other non-httpclient-related tasks running on threadpool, isn't it? So what this "fix" about? root cause is still there. It looks kind of masking the problem, but definitely not fixing it.

Creating a background session is a simple process and will ensure that, when the application is moved to the background, the request completes successfully (assuming no issues with the network etc..) and that the application will not freeze when it moves back to the foreground

Is it really so? As far as I know background downloads/uploads rely on native NSUrlSession API other that NSUrlSessionHandler uses: i.e downloadTask instead of dataTask. I doubt that background session makes any difference in terms of threadpool issue, app will freeze same way as with default session. So for now this "trick" looks for me as a way to get "non-auto-cancel" behavior back, just like a bool flag.

Add actually it is better to make explicit bool parameter for that to avoid confusion: background NSUrlSession has nothing to do with NSUrlHttpHandler and HttpClient. You can not use http client for background download on iOS

This check has no sense:

isBackgroundSession = !string.IsNullOrEmpty (configuration.Identifier);

It should be replaced with isAutoCancel parameter.

@spouliot
Copy link
Contributor

spouliot commented Aug 8, 2019

We're getting outside the original issue scope a bit :) The main issue was a change that caused an exception to be immediately thrown when an app was going to the background. Older version of XI did not behave the same way.

Part of this is semantics. Most people (but Apple) think of an application as being in the foreground or the background.

The reality is that this is not a boolean (fore/back) state. When an app goes to the background it will gradually lose services as the OS reclaims resources. So for a (short) while an application in the background can have its networking session works normally. After that it needs to follow Apple guidelines for background sessions.

I have not tried it myself so I don’t know if HttpClient w/NSUrlSessionHandler can be used for "background sessions". @mandel-macaque just left for vacation but I'm re-opening the issue so he'll reply once back to work.

AFAIK the above remains correct to avoid immediate termination of HttpClient connections when an app goes to the background (which was the original issue).

@spouliot spouliot reopened this Aug 8, 2019
@kamrankhalil
Copy link

@spouliot, @mandel-macaque I tried the fix mentioned "HttpClient with background session" to avoid this issue but that client doesn't quite work. Issue I experienced is that first call works and after that we get "Task was canceled" exception each time we send some request. It can be reproduced in this project I have attached.
https://drive.google.com/file/d/1-569EuvJwSDG04ElzlA7vrqvN-P1a9cN/view?usp=sharing

@johnthiriet
Copy link

@spouliot @mandel-macaque I could understand about the background issue but the fix cancels all network calls even if we just display the control center over the application. In this state, the application is not in background and it leads to a really weird user experience. I honestly think that if we could make a difference between a real "background" and an "inactive" state this would be a much better option.

@yaliashkevich
Copy link
Contributor

Agreed with @johnthiriet, this is extremely weird behavior, at least we need explicit way to configure it (not this absolutely unrelated background NSUrlSession "switch").

@marnilss
Copy link

@spouliot totally agree with you:

The reality is that this is not a boolean (fore/back) state. When an app goes to the background it will gradually lose services as the OS reclaims resources. So for a (short) while an application in the background can have its networking session works normally. After that it needs to follow Apple guidelines for background sessions.

As I understand it from reading iOS documentation, an App can actually continue to run in the background by creating background tasks UIApplication.SharedApplication.BeginBackgroundTask(..) (more like registering that you want to run code even in background). You can even check how much "background time" you have left: UIApplication.SharedApplication.BackgroundTimeRemaining.
So it definitely isn't as simple as on/off.
But I think that is simply for running code - not sure if that also applies to network transfers, i.e. if HttpClient would continue to work.

@mandel-macaque
Copy link
Member

@marnilss That is a very good question. The check is done when the app is running in the foreground and is moved to the background with a non background session. If the HttpClient is called within a BackgroundTask the HttpClient should be able to handle it. I will add a new property so that user can control that. That is, if you are indeed using the HttpClient in a background task, you should be able to skip the check.

mandel-macaque added a commit to mandel-macaque/xamarin-macios that referenced this issue Sep 5, 2019
…heck. xamarin#6443

Issue was reponed because users had a valid reason to want to bypass
this security check. The HttpClient should be able to work in a
background task. So we now provide a wya for user to explicitly ignore
the check.

Fixes: xamarin#6443
mandel-macaque added a commit that referenced this issue Sep 5, 2019
…heck. #6443 (#6918)

Issue was reponed because users had a valid reason to want to bypass
this security check. The HttpClient should be able to work in a
background task. So we now provide a way for users to explicitly ignore
the check.

Fixes: #6443
@marnilss
Copy link

marnilss commented Sep 6, 2019 via email

@claudioredi
Copy link

claudioredi commented May 15, 2020

@mandel-macaque: Is there any ticket created for the mono issue that triggered cancelling request? I would like to follow it so we know when we're 100% safe to use BypassBackgroundSessionCheck

I've set BypassBackgroundSessionCheck = true and tested the app very intensively with network link conditioner activated so I could simulate requests taking "long" to resolve... I didn't see any app freeze coming from background so far. I know this is not an 100% accurate indication of what will happen in production but I'm trying to understand if, at least for us, the remedy (cancelling the request so aggresively) is worse than the illness (being exposed to mono thread pool issues)

@mandel-macaque
Copy link
Member

@claudioredi There was an old issue in bugzilla about it and we do have #7080 to track this.

The old bugzilla issue is https://xamarin.github.io/bugzilla-archives/58/58633/bug.html#c7

The main issue is that this was an old bug and mono believe they fixed it and with the fix being confirmed we will remove the workaround or even set it to be disabled by default.

@chamons should we move this to be the default and skip the workaround?

@chamons
Copy link
Contributor

chamons commented May 18, 2020

We discussed this further, and the trouble is that we don't have solid enough automated/manual tests to be confident that such a default change won't "break the world".

We plan on documenting the opt-out flag in the 16.6 release notes and hope in the future to change the behavior.

@breyed
Copy link

breyed commented Nov 9, 2020

I have Xamarin.Forms code that calls BeginBackgroundTask via a Dependency Service and then calls HttpClient, which uses the NSURLConnection implementation. It used to work in the background, but no longer does for the reasons discussed in this issue.

Is there an equivalent to the new HttpClient (new NSUrlSessionHandler (configuration)) approach that works in cross-platform code? Should I create a Dependency Service that creates a configured HttpClient and returns it to the cross-platform code?

@chamons
Copy link
Contributor

chamons commented Nov 9, 2020

@breyed - Commenting on an issue more than half a year old isn't the best way to resolve your question.

You could try the discord or stack overflow.

With that out of the way: As NSUrlSessionHandler is a platform specific type, you may need to create it in platform specific code or using the project setting depending on usage.

@AnthonyIacono
Copy link

AnthonyIacono commented Nov 13, 2020

I am actually a little bit confused. As I understand it, we are configuring HttpClient to use NsUrlSession here:
<MtouchHttpClientHandler>NSUrlSessionHandler</MtouchHttpClientHandler>

And as I understand it, we want to use BypassBackgroundSessionCheck as true by default. Is this something we can set in the csproj?

We are using the latest version of Xamarin iOS and Xamarin Forms. Is this already the default?

For what it is worth, we have no observed the issue mentioned about. I just don't want any surprises.

To elaborate a little, we are using AddHttpClient from Microsoft.Extensions.DependencyInjection, and we would like to continue doing so if possible. We are using Polly like so:

            services
                .AddHttpClient<IApiDriver, ApiDriver>((clientServices, client) =>
                {
                    var apiConfiguration = clientServices.GetRequiredService<IApiConfiguration>();
                    client.BaseAddress = apiConfiguration.BaseUrl;
                })
                .AddPolicyHandler((policyServices, _) => HttpPolicyExtensions
                    .HandleTransientHttpError()
                    .WaitAndRetryAsync(Retries.Select(seconds => TimeSpan.FromSeconds(seconds))));

@ghost ghost locked as resolved and limited conversation to collaborators May 2, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug If an issue is a bug or a pull request a bug fix iOS Issues affecting Xamarin.iOS
Projects
None yet