-
Notifications
You must be signed in to change notification settings - Fork 520
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
Comments
Ahh I bet there's a relationship between these two bugs. |
Can someone confirm this as a bug? @spouliot ? |
stack trace
|
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 ? |
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. |
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. |
We're also experiencing this (on an iOS device):
Hoping for a quick resolution to this. Best regards, |
It looks like this behavior was introduced in this PR: 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 |
But it then means that backgrounding tasks using |
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. 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). Continue using the HttpClient as you have configuredIn 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. Using a NSUrlSessionHandler configured to work in the backgroundCreating 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)); SummaryWith 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 |
I this was not clear in the release notes, we should make sure that this does not happen again. Which I take full responsibility. |
@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. |
@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. |
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. |
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.
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: It should be replaced with |
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, @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. |
@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. |
Agreed with @johnthiriet, this is extremely weird behavior, at least we need explicit way to configure it (not this absolutely unrelated background NSUrlSession "switch"). |
@spouliot totally agree with you:
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. |
@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. |
…heck. dotnet#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: dotnet#6443
Has anyone actually tested successfully with using a HttpClient using NSUrlSessionHandler with a background session configuration?
Since the background session is meant to be used with the out-of-process Background Service in iOS and it will only work with downloading to file/uploading from file, and not writing to/reading from process memory, I can’t see how current NSUrlSessionHandler can work with a background session configuration.
When I tested an upload it also failed with strange error messages.
From: Manuel de la Pena <[email protected]>
Sent: den 5 september 2019 20:40
To: xamarin/xamarin-macios <[email protected]>
Cc: Marcus Nilsson <[email protected]>; Mention <[email protected]>
Subject: Re: [xamarin/xamarin-macios] NSUrlSession implementation of HttpClient throws TaskCanceledException immediately after backgrounding (#6443)
@marnilss<https://github.com/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.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#6443>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AGRGOJ3NY6M4FOSOEXW6N6LQIFG7FANCNFSM4H3ZAGTA>.
|
@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 I've set |
@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? |
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. |
I have Xamarin.Forms code that calls Is there an equivalent to the |
@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. |
I am actually a little bit confused. As I understand it, we are configuring HttpClient to use NsUrlSession here: And as I understand it, we want to use 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
|
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
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
Build Logs
buildlogs.txt
Example Project (If Possible)
TaskCanceledRepro.zip
The text was updated successfully, but these errors were encountered: