App crash due to NullReferenceException in NSUrlSessionHandlerDelegate.DidCompleteWithError #11799
Labels
bug
If an issue is a bug or a pull request a bug fix
iOS
Issues affecting iOS
mobcat
Issues affecting the mobile cat team
networking
If an issue or pull request is related to networking
support
The issue is related to support
Milestone
Comments
spouliot
added
iOS
c.c. @mandel-macaque
Can you provide us with a complete, symbolicated crash report ? Sadly the stack trace above is not telling us much about what's happening. Thanks! |
|
@spouliot @adams-family looking at the Xamarin.iOS version:
That points to commit: c51fabee8 that commits is earlier than the fix provided in #10230 If you look at the source code of the commit of the version you are using you can see that the fix I provided is not there. It is the same bug, but you are in a much older version which means that you do not have the fix.The current stable release is https://download.visualstudio.microsoft.com/download/pr/414d1675-ca0b-4b5b-8a4e-26b693883581/5e50228d1498caf85151b6d6cfec6fd9/xamarin.ios-14.20.0.1.pkg Please, make sure that you are running the latests stable and then we can re-visit the issue. (Edit: updated pkg to d16-10 which also has the fix) |
spouliot
added
support
|
I double-checked and the machine used to build the iOS version before releasing to the AppStore was this one: |
|
@spouliot - I have exported a crashreport from MS AppCenter, symbols were uploaded earlier therefore I hope it's useful to you: |
|
@adams-family so, from the version you have in the App Store, you have 14.14.2.5 with hash 3836759, looking in the repo, the code of that version points to https://github.com/xamarin/xamarin-macios/blob/3836759d4/src/Foundation/NSUrlSessionHandler.cs#L543 which does not have the fix. Please update and get back to us. |
|
@mandel-macaque Got it! Updating and will push a new version to the AppStore. I should know in about it week if it helped. |
spouliot
added
the
need-info
|
re-opening issue so this gets reviewed in 30 days (if not earlier) |
|
@spouliot I might have some news, not necessarily good one... I deployed version 1.1.8 of my app with the latest stable Xamarin.iOS framework: But still got the same exception from production (AppStore) which afterwards ended up with a crash:
Not sure if I can get you a more detailed report than this one:
BTW: Is there a way I can retrieve the Xamarin.iOS version (hash) during runtime? I would pass it to AppCenter crashlytics alongside with the app version. The only hackaround I found is this but I don't find it particularly reliable: https://stackoverflow.com/questions/54288683/how-to-programmatically-retrieve-the-version-information-of-visual-studio-for-ma |
Not really - but you should not as knowing the version only gives you a very limited amount of information, which is better than nothing but we settle for less ;-) I suggest you to generate msbuild binary logs (binlog files) when creating builds for submission. They are highly compressed, includes Xamarin.iOS version and a lot more very useful information. Keep the binlog along with your submitted .ipa and .dSYM. That way you can read [1] it (or share it with us) in the future and figure out a lot more than just the SDK version. |
Actually yes, we embed the Xamarin.iOS version (including hash) into the Info.plist when building the app. This is from an app I just on my hard drive: $ more bin/iPhone/Debug/testapp.app/Info.plist | grep com.xamarin.ios -A 4
<key>com.xamarin.ios</key>
<dict>
<key>Version</key>
<string>14.9.0.143 (main: 14ad1a122)</string>
</dict>
I heartily second this, the binlogs contain a wealth of information and are very useful for us in diagnosing all sorts of problems. |
|
Lets try to see the version used and I'l dig deeper into the issue. |
|
@rolfbjarne Thanks, the version number embedded in Info.plist is great, this way it can be forwarded to crash reporting 👍 @mandel-macaque Confirm that the version is
@spouliot "I suggest you to generate msbuild binary logs (binlog files)" - I agree, I did some research, can you please confirm to me that the following way is the right way to build binary logs on VS for Mac: |
Not sure you can do them from VS for Mac. You normally want to be able to read build logs if you're in an IDE. However you can build your msbuild /t:Build /p:Platform=iPhone /p:Configuration=Release /bl:app.binlog app.slnand you can use |
|
@adams-family it is indeed an issue since the changes in that hash are present: |
|
@mandel-macaque I just double checked that my app version The crash reason |
|
@adams-family yes, because this has not been that common until now, there might be changes in iOS that are generating a race condition. Do you know how to reproduce it? Can you show me how you instantiate the HttpClient? |
|
@mandel-macaque Confirming that with the latest version of our app being automatically deployed by the AppStore the number of exceptions increases, so the problem still persists. Sending samples of
using System;
using System.Net.Http;
using System.Threading.Tasks;
namespace HttpTest
{
class HttpTestPlain
{
protected HttpClient client = new HttpClient();
void Test()
{
var url = "https://httpbin.org/get"; // we don't actually use this url - this is just an example
Task.Run(async () =>
{
try
{
using (var client = new HttpClient())
{
using (var response = await client.GetAsync(url))
{
Console.WriteLine("Response code is: " + response.StatusCode);
}
}
}
catch (Exception e)
{
Console.WriteLine(e.ToString());
}
});
}
}
}
using System;
using System.Collections.Generic;
using System.Net.Http;
using System.Threading.Tasks;
using Polly;
namespace HttpTest
{
class HttpTestWithPolly
{
protected HttpClient client = new HttpClient();
void Test()
{
var url = "https://httpbin.org/post"; // we don't actually use this url - this is just an example
Task.Run(async () =>
{
try
{
var postData = new List<KeyValuePair<string, string>>();
postData.Add(new KeyValuePair<string, string>("v", "1"));
HttpContent content = new FormUrlEncodedContent(postData);
var result = await Policy
.Handle<HttpRequestException>(ex => !ex.Message.ToLower().Contains("404"))
.WaitAndRetryAsync
(
retryCount: 3,
sleepDurationProvider: retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt)),
onRetry: (ex, time) =>
{
Console.WriteLine("Something went wrong: {ex.Message}");
}
)
.ExecuteAsync(async () =>
{
var response = await client.PostAsync(url, content);
Console.WriteLine("Response code is: " + response.StatusCode);
return true;
});
}
catch (Exception e)
{
Console.WriteLine(e.ToString());
}
});
}
}
}Unfortunately from the crash logs it's unclear which one causes the crash, however I can say that we do have more places using the Polly wrapper than those that don't. |
|
@mandel-macaque Confirming that it's still happening on: Stacktrace: |
|
@mandel-macaque Just confirming that I received a crash report from |
|
Hmmm we might have an issue with Polly, I di don't thing about that, i'll take a look with that exact sample. |
|
@mandel-macaque This issue is still a big problem in our app. In #9132 you said there was a fix made in branch d16-10. We have installed this version of Xamarin.iOS in Visual Studio for Mac: This is a callstack from AppCenter of one of our users: https://github.com/xamarin/xamarin-macios/blob/d16-10/src/Foundation/NSUrlSessionHandler.cs So, I assume like in #9132 the CancellationTokenSource is null and therefore the crash. Why not check for null here? BTW, why I can see the path of the developers workspace in official version 14.20.0.24? (/Users/builder/azdo/_work/....) In the AppCenter callstacks of the older version of Xamarin.iOS 13.20.2.2 I saw the version string in the path: [UPDATE] |
|
@
Is this an improvement over previous versions? |
|
@hjharvey-MSFT i would say yes - even through the beta phase we saw higher numbers and pointing to different NSURLSession methods. |
|
I am also seeing a ton of app crashes after migrating to .NET7 and doing concurrent network requests. Stack traces look a bit different (I am also using AppCenter, not Sentry). I also reported this here. Might be the same bug despite the different stack traces? Edit: was able to extract this data out of one stack trace, maybe it contains and useful info: |
|
@hjharvey-MSFT having a look in Xcode for crashes, we got 2 reports for NSURLSessionDelegateWrapper, one with 9 crashes and the other one with23 |
|
Unfortunately with the current information we have there is really not a good idea on the why this is happening. We would need a way to reproduce this issue in order to continue investigating this. |
|
On one of the referenced issues that is closed, i commented about triggering this crash by hitting a breakpoint where i was making a network request and letting it stay on the breakpoint for some time. |
|
I am doing some symbolication at the moment and i keep seeing this line: |
|
@narciszait would you be able to provide a sample and instructions on how to repro the issue? |
|
@hjharvey-MSFT unfortunately no - i have no idea on how to reproduce it, hence i cannot make a sample. Like i said, i managed once in a blue moon to get the crash when hitting a breakpoint where i was making a network request and letting it stay on the breakpoint for some time. |
|
We are seeing this crash from time to time as well, I think it happens in cases when the app was in the background for a long time. Info.plist: Crash from AppCenter: |
|
We are also seeing crash reports with the same stack trace in AppCenter: This issue might be hard to reproduce because it requires very precise timings, but I have a theory about possible root cause: According to the source code of the Inflight data is removed whenever original managed request is cancelled: So, one of possible reproduction scenarios is following: original managed request is being cancelled shortly after |
|
@hjharvey-MSFT any update on this, given the hypothesis from @TimofeyBurak above? |
|
This is still a thing, we are getting tons of those after updating to net7. |
|
I was able to reproduce the crash locally 4 times within just a couple of minutes using this example: I also noticed several of those logs, which appear to be unrelated, but perhaps should be addressed as well: |
I can reproduce crashes too. I'll have a look. |
|
It seems this is the sequence of events for the ObjectDisposedException:
Full stack traces: @mandel-macaque it seems an easy fix would be to just not dispose the CancellationTokenSource, the GC will eventually do it. Although MSDN says quite clearly:
The amount of entry points into this code makes it quite complex to keep it |
rolfbjarne
added a commit
to rolfbjarne/xamarin-macios
that referenced
this issue
Mar 18, 2024
… NSUrlSessionHandler.
Fixes these warnings:
[API] API MISUSE: NSURLSession delegate System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: <System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: 0x600000676c60> (0x600000676c60)
[API] API MISUSE: dataTask:didReceiveResponse:completionHandler: completion handler not called
[API] API MISUSE: NSURLSession delegate System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: <System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: 0x600000676c60> (0x600000676c60)
[API] API MISUSE: dataTask:willCacheResponse:completionHandler: completion handler not called
[API] API MISUSE: NSURLSession delegate System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: <System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: 0x6000002cffa0> (0x6000002cffa0)
[API] API MISUSE: dataTask:willCacheResponse:completionHandler: completion handler not called
which is printed numerous times with the test case here:
xamarin#11799 (comment)
Fix in progress: #20326. |
rolfbjarne
added a commit
to rolfbjarne/xamarin-macios
that referenced
this issue
Mar 18, 2024
…nHandler's InflightData. Fixes xamarin#11799. There's a random ObjectDisposedException occurring, which seems to have this sequence of events: 1. The request is sent. 2. The request is cancelled. 3. The InflightData instance is removed here: https://github.com/xamarin/xamarin-macios/blob/c3437328bf2c02d03a6d4dc4e752ef8ac927ca4e/src/Foundation/NSUrlSessionHandler.cs#L545 3a. The InflightData is disposed: https://github.com/xamarin/xamarin-macios/blob/c3437328bf2c02d03a6d4dc4e752ef8ac927ca4e/src/Foundation/NSUrlSessionHandler.cs#L224 3a. The InflightData.CancellationTokenSource instance is disposed: https://github.com/xamarin/xamarin-macios/blob/c3437328bf2c02d03a6d4dc4e752ef8ac927ca4e/src/Foundation/NSUrlSessionHandler.cs#L1168 4. Data comes in (in the DidReceiveData callback), and SetResponse is called: https://github.com/xamarin/xamarin-macios/blob/c3437328bf2c02d03a6d4dc4e752ef8ac927ca4e/src/Foundation/NSUrlSessionHandler.cs#L932 4a. The SetResponse method tries to use InflightData.CancellationTokenSource, and that throws an ObjectDisposedException: https://github.com/xamarin/xamarin-macios/blob/c3437328bf2c02d03a6d4dc4e752ef8ac927ca4e/src/Foundation/NSUrlSessionHandler.cs#L970 Full stack traces: ``` InflightData disposed: at System.Net.Http.NSUrlSessionHandler.InflightData.Dispose(Boolean disposing) in xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:line 1168 at System.Net.Http.NSUrlSessionHandler.InflightData.Dispose() in xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:line 1160 at System.Net.Http.NSUrlSessionHandler.RemoveInflightData(NSUrlSessionTask task, Boolean cancel) in xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:line 224 at System.Net.Http.NSUrlSessionHandler.<>c__DisplayClass58_0.<SendAsync>b__0() in xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:line 545 at System.Threading.CancellationToken.<>c.<Register>b__12_0(Object obj) at System.Threading.CancellationTokenSource.Invoke(Delegate d, Object state, CancellationTokenSource source) at System.Threading.CancellationTokenSource.CallbackNode.<>c.<ExecuteCallback>b__9_0(Object s) at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) at System.Threading.CancellationTokenSource.CallbackNode.ExecuteCallback() at System.Threading.CancellationTokenSource.ExecuteCallbackHandlers(Boolean throwOnFirstException) at System.Threading.CancellationTokenSource.NotifyCancellation(Boolean throwOnFirstException) at System.Threading.CancellationTokenSource.LinkedNCancellationTokenSource.<>c.<.cctor>b__4_0(Object s) at System.Threading.CancellationTokenSource.Invoke(Delegate d, Object state, CancellationTokenSource source) at System.Threading.CancellationTokenSource.CallbackNode.ExecuteCallback() at System.Threading.CancellationTokenSource.ExecuteCallbackHandlers(Boolean throwOnFirstException) at System.Threading.CancellationTokenSource.NotifyCancellation(Boolean throwOnFirstException) at System.Threading.CancellationTokenSource.TimerCallback(Object state) at System.Threading.TimerQueueTimer.CallCallback(Boolean isThreadPool) at System.Threading.TimerQueueTimer.Fire(Boolean isThreadPool) at System.Threading.TimerQueue.FireNextTimers() at System.Threading.TimerQueue.System.Threading.IThreadPoolWorkItem.Execute() at System.Threading.ThreadPoolWorkQueue.DispatchItemWithAutoreleasePool(Object workItem, Thread currentThread) at System.Threading.ThreadPoolWorkQueue.Dispatch() at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart() at System.Threading.Thread.StartCallback() ``` ``` ObjectDisposedException: *** Terminating app due to uncaught exception 'System.ObjectDisposedException', reason: 'The CancellationTokenSource has been disposed. (System.ObjectDisposedException) at System.Net.Http.NSUrlSessionHandler.NSUrlSessionHandlerDelegate.SetResponse(InflightData inflight) in xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:line 970 at System.Net.Http.NSUrlSessionHandler.NSUrlSessionHandlerDelegate.DidReceiveData(NSUrlSession session, NSUrlSessionDataTask dataTask, NSData data) in xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:line 932 ' *** First throw call stack: ( 0 CoreFoundation 0x0000000186e72ccc __exceptionPreprocess + 176 1 libobjc.A.dylib 0x000000018695a788 objc_exception_throw + 60 2 nsurlsessionhandler 0x0000000106d4562c xamarin_process_managed_exception + 1052 3 nsurlsessionhandler 0x00000001071a26c4 _ZL31native_to_managed_trampoline_53P11objc_objectP13objc_selectorPP11_MonoMethodS0_S0_S0_j + 1028 4 nsurlsessionhandler 0x00000001071ca8e0 -[System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate URLSession:dataTask:didReceiveData:] + 68 5 CFNetwork 0x000000018c0fe28c CFURLCredentialStorageCopyAllCredentials + 61900 6 libdispatch.dylib 0x0000000186b6c750 _dispatch_call_block_and_release + 32 7 libdispatch.dylib 0x0000000186b6e3e8 _dispatch_client_callout + 20 8 libdispatch.dylib 0x0000000186b75a14 _dispatch_lane_serial_drain + 748 9 libdispatch.dylib 0x0000000186b76578 _dispatch_lane_invoke + 432 10 libdispatch.dylib 0x0000000186b812d0 _dispatch_root_queue_drain_deferred_wlh + 288 11 libdispatch.dylib 0x0000000186b80b44 _dispatch_workloop_worker_thread + 404 12 libsystem_pthread.dylib 0x0000000186d1b00c _pthread_wqthread + 288 13 libsystem_pthread.dylib 0x0000000186d19d28 start_wqthread + 8 ``` Disposing the CancellationTokenSource would be ideal, but the additional complexity this would entail makes me question whether it's worth it: the amount of entry points into this code makes it quite complex to keep track of what the request has done, what it's doing and what's pending when things can happen simultaneously on multiple threads at the same time, and keeping the code thread-safe, and at the same time not accidentally not run into deadlocks. So it seems an easy fix would be to just not dispose the CancellationTokenSource, the GC will eventually do it, even though MSDN [says][1] quite clearly: > Always call Dispose before you release your last reference to the CancellationTokenSource. Otherwise, the resources it is using will not be freed until the garbage collector calls the CancellationTokenSource object's Finalize method. An additional note is that a network request's resource usage would already be dwarfing any memory usage by the CancellationTokenSource, so it's unlikely this will show up in any profiles. So with this change we won't be disposing the CancellationTokenSource anymore. Fixes xamarin#11799. [1]: https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtokensource.dispose?view=net-8.0
rolfbjarne
added
the
networking
rolfbjarne
added a commit
that referenced
this issue
Mar 19, 2024
… NSUrlSessionHandler. (#20326) Fixes these warnings: [API] API MISUSE: NSURLSession delegate System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: <System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: 0x600000676c60> (0x600000676c60) [API] API MISUSE: dataTask:didReceiveResponse:completionHandler: completion handler not called [API] API MISUSE: NSURLSession delegate System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: <System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: 0x600000676c60> (0x600000676c60) [API] API MISUSE: dataTask:willCacheResponse:completionHandler: completion handler not called [API] API MISUSE: NSURLSession delegate System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: <System_Net_Http_NSUrlSessionHandler_NSUrlSessionHandlerDelegate: 0x6000002cffa0> (0x6000002cffa0) [API] API MISUSE: dataTask:willCacheResponse:completionHandler: completion handler not called which is printed numerous times with the test case here: #11799 (comment)
|
@rolfbjarne |
The networking code is rather sensitive, in that any changes has a high potential for breaking someone. Coupled with the fact that these changes weren't entirely trivial, at the moment we're leaning towards not backporting them to .NET 8. However, these changes are included in .NET 9 preview 3, so if you want to try the preview, and see if your issues are fixed, then that would be great (and a successful test might also make us reconsider backporting the fixes). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
IMPORTANT:
I think this problem will be somewhat related to: #9132
Steps to Reproduce
I happens on iOS, latest versions (latest iOS, latest Xamarin) very often in production (tens of crashes per day on a 2k user basis), randomly. It's hard to reproduce it in the debugger.
Expected Behavior
Thanks to the try/catch block this code should never cause an app crash.
Actual Behavior
The app crashes. Here is an AppCenter stacktrace:
Environment
Example Project (If Possible)
I don't think it's possible to create one because I'd need to roll it out to at least a thousand users.
The text was updated successfully, but these errors were encountered: