App crash due to NullReferenceException in NSUrlSessionHandlerDelegate.SetResponse #9132
Comments
whitneyschmidt
added
the
need-info
|
Thank you for your feedback! For us to investigate this further, could you please provide your full build logs, crash reports (if any), and small sample project. The sample is the best way for us to investigate and reproduce the issue. To get full build logs just set the log verbosity to diagnostic at the following locations:
We look forward to hearing from you! |
|
It's not very reproducible, so I'm not sure if a sample project would do any good - I've only seen it in person once, which allowed me to narrow down the null reference. I can contrive a simple app that uses an HttpClient with a native handler to hit an API if it's necessary in order for the issue to be investigated. I've attached full build logs from a local build along with crash reports from our production app. I've included one report from another NSUrlSessionHandler NRE crash, this one in |
|
@StefAnglr Thanks for investigating and attaching the crash logs. A sample project replicating the conditions of the crash would be useful. Even if we can't reproduce, it'll at least give a better idea of what conditions make the crash possible. For visibility: |
|
We have not received the requested information. If you are still experiencing this issue please provide all the requested information then click the Reopen Issue button. Thanks! |
|
Sorry for the delay on this. Attached is a simple one-screen iOS app that uses HttpClient in a similar manner to our application. We implement timeouts using our own CancellationTokenSource due to the native HttpClient implementations throwing different platform-specific exceptions on timeouts (mainly an issue on Android). In the meantime, we have copied the NSUrlSessionHandler code into our own application and added the exception handling ourselves to prevent the crashes from happening again. Once that build is released to the public we may have more useful logging from those handled exceptions. |
|
I don't see a "Reopen Issue" button - I'm guessing I don't have the permissions for that. |
|
@StefAnglr strange, you should have the rights to reopen. Anyway, I'll do it. Regarding the issue, it looks very interesting, I have a few questions about the repo application:
I will get to this ASAP, do please take into consideration that we are working on the Xcode 12 changes and I might take a little longer than I would like. PS: @whitneyschmidt I'll take over this. |
mandel-macaque
added
the
networking
|
Thanks for the reply. I understand if this is low priority due to the low reproduction rate, and I think our workaround should prevent further crashes once released, but I appreciate whatever attention you can give it.
|
rolfbjarne
removed
the
need-info
|
Here's a few more crash logs from our latest build. I tried to use our own copy of NSUrlSessionHandler with these exceptions caught, but I've either misconfigured something, or a dependency is still using the Xamarin version. |
|
In Microsoft AppCenter, we also see this NullReferenceException crash in our app. The crash points into NSUrlSessionHandler when accessing the CancellationTokenSource of this "inflight" object. This is the source code at the crash location in /Library/Frameworks/Xamarin.iOS.framework/Versions/13.20.2.2/src/Xamarin.iOS/Foundation/NSUrlSessionHandler.cs (line 663): Why there is no "null" check here and in all other places where the CancellationTokenSource is accessed? In our Xamarin Forms shared code in an utils class we simply create a HttpClient instance and send a request: I also tried to put a try/catch around the HttpClient.SendAsync() but this kind of crash will not be catched? Don't know why. How can I avoid this crash? Can I do something in my application code or have I wait till the issue is fixed in a new release? |
|
I believe it's not possible to catch the exception in your own code because the exception is thrown in a networking callback. We've tried to work around the issue by using our own customized version of NSUrlSessionHandler, but we have dependencies that don't easily support customizing their HttpClients. |
|
This NullReferenceException in DidCompleteWithError() which I reported above is more and more a problem for us. The number of crashes is increasing in AppCenter. For info, we upgraded from Xamarin.iOS 13.6.0.12 to 13.20.2.2. @StefAnglr: You said you wrote a customized NSUrlSessionHandler? How did you do this? I also think about to override the DidCompleteWithError() callback and add this null-check by myself, but I don't know how. Would be glad if someone has a workaround for this. |
|
I made some analyzing and want to share the infos here. I duplicated the file "NSUrlSessionHandler.cs" and renamed the class to "MyNSUrlSessionHandler" and call the HttpClient constructor with this copied handler. This way it is possible to debug the code. I wanted to find out why the CancellationTokenSource is "null" at some places which then leads to the NRE in SetResponse() or DidCompleteWithError(). Inside the GetInflightData() method the inflight is fetched by using the task object. We see that the "IsCancellationRequested" flag is "True" and the CancellationTokenSource is still valid. Because the flag is true a Cancel() will be called on the task instance. When I step further in the debugger, you see that the CancellationTokenSource is now "null". I assume because of the Cancel() the CancellationTokenSource was disposed and set to null): After the GetInflightData() returns the inflight in DidReceiveResponse() there is an access to inflight.CancellationTokenSource in line 644 in the next screenshot, which leads to a NullReferenceException (line 687): Because the DidReceiveResponse() method has a try/catch this NRE is handled and the app doesn't crash. But what is when the DidCompleteWithError() or the DidReceiveData() is called directly without DidReceiveResponse()? In DidCompleteWithError() and DidReceiveData() the method GetInflightData() is called to get the inflight, the Cancel() will be called inside GetInflightData() which leads to a "null" in CancellationTokenSource. These 2 methods have no try/catch and so the SetResponse() call inside DidReceiveData() or the line "inflight.CancellationTokenSource.Cancel();" inside DidCompleteWithError() will throw a NullReferenceException and the app crashes. @mandel-macaque Can you please have a closer look into this issue? Perhaps my infos above help you. The question is why your code expect that the CancellationTokenSource is never "null" when we see here that it will be set to "null" whenever the task will be cancelled? |
|
Been hunting this for a while and found this thread. We have around 50 crashes per day because of this. A solution would be most welcome |
|
@SuperMario77 Thanks for investigating this and sorry I missed your question earlier. To try to work around the crash, you can add the exception handling to your Then in your shared code, you can resolve |
|
@mandel-macaque @rolfbjarne BUT there are still 3rd party libraries in our app like MSAL library (Microsoft Identity Client) for doing Azure AD authentication which still use the HttpClient with native NSUrlSessionHandler. And the NREs in NSUrlSessionHandler still happen a lot, which means a crash of the app, which means unsatisfied customers. Please take again a look into this issue. |
|
@mandel-macaque |
|
Sorry for the delay. I had to work on other issues but I have bump this to the top of my queue. Will get a fix ASAP. |
…ixes dotnet#9132 This is an interesting issue that happens in the following scenario: 1. A request is performed that will take some extra time to get a response or an error. 2. The request is cancelled BEFORE the delegate methods DidReceiveResponse, DidReceiveData or DidCompleteWithError and called. Yet we have not yet fully canceled the native request. It does take some time to do so, that is why we have a 'NSURLSessionTaskStateCanceling' and not 'NSURLSessionTaskStateCancelled' The issue happens because the GetInflightData checks if the task has been canceled. In that method we have the following: ```csharp if (inflight.CancellationToken.IsCancellationRequested) task?.Cancel (); return inflight; ``` The call of the Cancel method in the NSData task has as ripple effect which is the execution of the following callback: ```csharp cancellationToken.Register (() => { RemoveInflightData (dataTask); tcs.TrySetCanceled (); }); ``` Which calls: ```csharp void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) { lock (inflightRequestsLock) { if (inflightRequests.TryGetValue (task, out var data)) { if (cancel) data.CancellationTokenSource.Cancel (); data.Dispose (); inflightRequests.Remove (task); } // do we need to be notified? If we have not inflightData, we do not if (inflightRequests.Count == 0) RemoveNotification (); } if (cancel) task?.Cancel (); task?.Dispose (); } ``` The above call does call Dispose and Dipose in the inflight data does: ```csharp if (disposing) { if (CancellationTokenSource != null) { CancellationTokenSource.Dispose (); CancellationTokenSource = null; } } ``` If we follow these set of events for example in the DidRecieveResponse: ```chsarp [Preserve (Conditional = true)] public override void DidReceiveResponse (NSUrlSession session, NSUrlSessionDataTask dataTask, NSUrlResponse response, Action<NSUrlSessionResponseDisposition> completionHandler) { var inflight = GetInflightData (dataTask); if (inflight == null) return; try { var urlResponse = (NSHttpUrlResponse)response; var status = (int)urlResponse.StatusCode; var content = new NSUrlSessionDataTaskStreamContent (inflight.Stream, () => { if (!inflight.Completed) { dataTask.Cancel (); } inflight.Disposed = true; inflight.Stream.TrySetException (new ObjectDisposedException ("The content stream was disposed.")); sessionHandler.RemoveInflightData (dataTask); }, inflight.CancellationTokenSource.Token); ``` If can happen that, when we get the delegate call to the method, the request has been cancelled. That means that we are in the precise state in which we do not longer care about the response BUT we did get a infligh data reference.. that HAS BEEN DIPOSED!!! this later gives a NRE in several places. The correct way is to return null from the GetInflighData method if we have been cancelled, this makes sense because if we cancelled we do not care about the execution of any of the methods that required the data since it has been disposed!
|
@SuperMario77 @StefAnglr @ThomasHoest I have created a fix for the issue. If you check the PR you will see an explanation of why we had a crash. tldr; The cancellation of the task in the inflight data had a ripple effect that disposed the instance of the infligh data. If the task is cancelled the result of that method should be null to ignore the rest of the delegate methods. |
|
Looks like a fun bug to track down. Thanks for investigating! |
|
@StefAnglr @ThomasHoest @SuperMario77 You can find the following packages with the fix: Can you please test and confirm is fixed and I'll download the fix and will backport to the current stable branch. |
…ixes #9132 (#10230) This is an interesting issue that happens in the following scenario: 1. A request is performed that will take some extra time to get a response or an error. 2. The request is cancelled BEFORE the delegate methods DidReceiveResponse, DidReceiveData or DidCompleteWithError and called. Yet we have not yet fully canceled the native request. It does take some time to do so, that is why we have a 'NSURLSessionTaskStateCanceling' and not 'NSURLSessionTaskStateCancelled' The issue happens because the GetInflightData checks if the task has been canceled. In that method we have the following: ```csharp if (inflight.CancellationToken.IsCancellationRequested) task?.Cancel (); return inflight; ``` The call of the Cancel method in the NSData task has as ripple effect which is the execution of the following callback: ```csharp cancellationToken.Register (() => { RemoveInflightData (dataTask); tcs.TrySetCanceled (); }); ``` Which calls: ```csharp void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) { lock (inflightRequestsLock) { if (inflightRequests.TryGetValue (task, out var data)) { if (cancel) data.CancellationTokenSource.Cancel (); data.Dispose (); inflightRequests.Remove (task); } // do we need to be notified? If we have not inflightData, we do not if (inflightRequests.Count == 0) RemoveNotification (); } if (cancel) task?.Cancel (); task?.Dispose (); } ``` The above call does call Dispose and Dipose in the inflight data does: ```csharp if (disposing) { if (CancellationTokenSource != null) { CancellationTokenSource.Dispose (); CancellationTokenSource = null; } } ``` If we follow these set of events for example in the DidRecieveResponse: ```chsarp [Preserve (Conditional = true)] public override void DidReceiveResponse (NSUrlSession session, NSUrlSessionDataTask dataTask, NSUrlResponse response, Action<NSUrlSessionResponseDisposition> completionHandler) { var inflight = GetInflightData (dataTask); if (inflight == null) return; try { var urlResponse = (NSHttpUrlResponse)response; var status = (int)urlResponse.StatusCode; var content = new NSUrlSessionDataTaskStreamContent (inflight.Stream, () => { if (!inflight.Completed) { dataTask.Cancel (); } inflight.Disposed = true; inflight.Stream.TrySetException (new ObjectDisposedException ("The content stream was disposed.")); sessionHandler.RemoveInflightData (dataTask); }, inflight.CancellationTokenSource.Token); ``` If can happen that, when we get the delegate call to the method, the request has been cancelled. That means that we are in the precise state in which we do not longer care about the response BUT we did get a infligh data reference.. that HAS BEEN DIPOSED!!! this later gives a NRE in several places. The correct way is to return null from the GetInflighData method if we have been cancelled, this makes sense because if we cancelled we do not care about the execution of any of the methods that required the data since it has been disposed! Co-authored-by: Rolf Bjarne Kvinge <[email protected]>
|
@mandel-macaque i looked at the latest release and i don't see this item on it. Do you know when it will be on there? |
|
@ahhassan3 I need to check with the release manager, it was landed in main but was not bacported. @dalexsoto do you have any ideas? |
|
@mandel-macaque i just looked at the source files for the latest xamarin ios release (xamarin-ios-14.14.2.5) and the fix wasn't on there. i don't understand why this isn't getting released |
|
Hi @mandel-macaque, any update on how to resolve this crash issue please? any fix available? |
|
This is a long-standing issue for us too. Multiple crashes per day among our userbase, which of course are interpreted as instability in our software (I suppose they are correct). Since a fix has obviously been engineered, why is it not in our hands? |
…ixes dotnet#9132 (dotnet#10230) This is an interesting issue that happens in the following scenario: 1. A request is performed that will take some extra time to get a response or an error. 2. The request is cancelled BEFORE the delegate methods DidReceiveResponse, DidReceiveData or DidCompleteWithError and called. Yet we have not yet fully canceled the native request. It does take some time to do so, that is why we have a 'NSURLSessionTaskStateCanceling' and not 'NSURLSessionTaskStateCancelled' The issue happens because the GetInflightData checks if the task has been canceled. In that method we have the following: ```csharp if (inflight.CancellationToken.IsCancellationRequested) task?.Cancel (); return inflight; ``` The call of the Cancel method in the NSData task has as ripple effect which is the execution of the following callback: ```csharp cancellationToken.Register (() => { RemoveInflightData (dataTask); tcs.TrySetCanceled (); }); ``` Which calls: ```csharp void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) { lock (inflightRequestsLock) { if (inflightRequests.TryGetValue (task, out var data)) { if (cancel) data.CancellationTokenSource.Cancel (); data.Dispose (); inflightRequests.Remove (task); } // do we need to be notified? If we have not inflightData, we do not if (inflightRequests.Count == 0) RemoveNotification (); } if (cancel) task?.Cancel (); task?.Dispose (); } ``` The above call does call Dispose and Dipose in the inflight data does: ```csharp if (disposing) { if (CancellationTokenSource != null) { CancellationTokenSource.Dispose (); CancellationTokenSource = null; } } ``` If we follow these set of events for example in the DidRecieveResponse: ```chsarp [Preserve (Conditional = true)] public override void DidReceiveResponse (NSUrlSession session, NSUrlSessionDataTask dataTask, NSUrlResponse response, Action<NSUrlSessionResponseDisposition> completionHandler) { var inflight = GetInflightData (dataTask); if (inflight == null) return; try { var urlResponse = (NSHttpUrlResponse)response; var status = (int)urlResponse.StatusCode; var content = new NSUrlSessionDataTaskStreamContent (inflight.Stream, () => { if (!inflight.Completed) { dataTask.Cancel (); } inflight.Disposed = true; inflight.Stream.TrySetException (new ObjectDisposedException ("The content stream was disposed.")); sessionHandler.RemoveInflightData (dataTask); }, inflight.CancellationTokenSource.Token); ``` If can happen that, when we get the delegate call to the method, the request has been cancelled. That means that we are in the precise state in which we do not longer care about the response BUT we did get a infligh data reference.. that HAS BEEN DIPOSED!!! this later gives a NRE in several places. The correct way is to return null from the GetInflighData method if we have been cancelled, this makes sense because if we cancelled we do not care about the execution of any of the methods that required the data since it has been disposed! Co-authored-by: Rolf Bjarne Kvinge <[email protected]>
…ixes #9132 (#10230) (#10930) This is an interesting issue that happens in the following scenario: 1. A request is performed that will take some extra time to get a response or an error. 2. The request is cancelled BEFORE the delegate methods DidReceiveResponse, DidReceiveData or DidCompleteWithError and called. Yet we have not yet fully canceled the native request. It does take some time to do so, that is why we have a 'NSURLSessionTaskStateCanceling' and not 'NSURLSessionTaskStateCancelled' The issue happens because the GetInflightData checks if the task has been canceled. In that method we have the following: ```csharp if (inflight.CancellationToken.IsCancellationRequested) task?.Cancel (); return inflight; ``` The call of the Cancel method in the NSData task has as ripple effect which is the execution of the following callback: ```csharp cancellationToken.Register (() => { RemoveInflightData (dataTask); tcs.TrySetCanceled (); }); ``` Which calls: ```csharp void RemoveInflightData (NSUrlSessionTask task, bool cancel = true) { lock (inflightRequestsLock) { if (inflightRequests.TryGetValue (task, out var data)) { if (cancel) data.CancellationTokenSource.Cancel (); data.Dispose (); inflightRequests.Remove (task); } // do we need to be notified? If we have not inflightData, we do not if (inflightRequests.Count == 0) RemoveNotification (); } if (cancel) task?.Cancel (); task?.Dispose (); } ``` The above call does call Dispose and Dipose in the inflight data does: ```csharp if (disposing) { if (CancellationTokenSource != null) { CancellationTokenSource.Dispose (); CancellationTokenSource = null; } } ``` If we follow these set of events for example in the DidRecieveResponse: ```chsarp [Preserve (Conditional = true)] public override void DidReceiveResponse (NSUrlSession session, NSUrlSessionDataTask dataTask, NSUrlResponse response, Action<NSUrlSessionResponseDisposition> completionHandler) { var inflight = GetInflightData (dataTask); if (inflight == null) return; try { var urlResponse = (NSHttpUrlResponse)response; var status = (int)urlResponse.StatusCode; var content = new NSUrlSessionDataTaskStreamContent (inflight.Stream, () => { if (!inflight.Completed) { dataTask.Cancel (); } inflight.Disposed = true; inflight.Stream.TrySetException (new ObjectDisposedException ("The content stream was disposed.")); sessionHandler.RemoveInflightData (dataTask); }, inflight.CancellationTokenSource.Token); ``` If can happen that, when we get the delegate call to the method, the request has been cancelled. That means that we are in the precise state in which we do not longer care about the response BUT we did get a infligh data reference.. that HAS BEEN DIPOSED!!! this later gives a NRE in several places. The correct way is to return null from the GetInflighData method if we have been cancelled, this makes sense because if we cancelled we do not care about the execution of any of the methods that required the data since it has been disposed! Co-authored-by: Rolf Bjarne Kvinge <[email protected]>
|
Think I've jsut had this issue occur myself, but would be good to know if thats the case. Had the following Exception occur, I think when I cancelled out a REST request. This the same issue? I'm on the latest Stable Visual Studio branch Anyone able to confirm if this is the same area/line/crash as being reported, hard to tell if the above is fixed in an actual release as people are reporting it isn't but the ticket is closed several months ago. Cheers for any advice 👍 |
|
Which is this closed and obviously not fixed? |
|
2.500 crashes on this every month. When will the fix be released? |
|
This is so not closed/released... |
|
Any update on when this fix will be released? |
|
Having issues here too - a number of crashes every day - would be great to have a fix ASAP |
|
+1 Having exactly the same issue, lots of crashes daily because of this error - this needs to be fixed. |
|
Also seeing many NRE that are very likely to be caused by this issue. |
|
This issue is closed because the issue has been fixed and released. The only person that has provided a Xamarin.iOS version has been @IainS1986 with version 14.14.2.5, hash 3836759 which as you can see in the source code https://github.com/xamarin/xamarin-macios/blob/3836759d4/src/Foundation/NSUrlSessionHandler.cs#L543 does not have the fix. The d16-9 branch is not longer the stable branch, since we moved to the d16-10 branch. @mduchev @alexanderdibenedetto @tipa the latests release of Xamarin.iOS is https://download.visualstudio.microsoft.com/download/pr/414d1675-ca0b-4b5b-8a4e-26b693883581/5e50228d1498caf85151b6d6cfec6fd9/xamarin.ios-14.20.0.1.pkg Please test the stable version, if the issue reproduces, do not simple add a +1, that is not giving me enough information to do any work, even though you want to help it does the opposite :( If the above pkgs does not have the issue resolved, please provided the full environment information and the symbolicated exception. @adams-family as I have stated in your issue, the version you are using is 14.8.0.3 (c51fabe) which is much older than the fix (10th of December when the fix landed on the 22 second of March). @IainS1986 Please update to the provided pkg, I missed your question because closed issues are muted by default, please next time feel free to @ me directly in the issue so that I can responde faster, I take the blame for not looking at conversations in closed issues. (Edit: updated the pkg to d16-10 which is the new stable) |
|
@mandel-macaque Thanks for your quick response, I've posted a comment in the issue I've created. |
|
I'm having the same issue, Xamarin.iOS version 15.2. Only affects 1% of users per week - probably one in a hundred requests. I'll see if I can make a sample project after the new year |
|
Also still seeing the issues in 15.x. It is not fixed. 1% seems about right looking at our crash numbers |
|
Had this happen to me today |
|
Still our highest cause for crashes on iOS. About 250 per day. This should be re-opened. |
|
Still here. @mandel-macaque what do you need to look into this again? Main Global terminating exception: Object reference not set to an instance of an object stack at System.Net.Http.NSUrlSessionHandler+NSUrlSessionHandlerDelegate.DidCompleteWithError (Foundation.NSUrlSession session, Foundation.NSUrlSessionTask task, Foundation.NSError error) <0x101ffd2c0 + 0x00074> in <a7229a51752a4b23a2321c4878d035f4#b2bc9717d735d13fb9be7e6434b655da>:0 System.NullReferenceException Object reference not set to an instance of an object at System.Net.Http.NSUrlSessionHandler+NSUrlSessionHandlerDelegate.DidCompleteWithError (Foundation.NSUrlSession session, Foundation.NSUrlSessionTask task, Foundation.NSError error) <0x101ffd2c0 + 0x00074> in <a7229a51752a4b23a2321c4878d035f4#b2bc9717d735d13fb9be7e6434b655da>:0 | System.NullReferenceException Object reference not set to an instance of an object at System.Net.Http.NSUrlSessionHandler+NSUrlSessionHandlerDelegate.DidCompleteWithError (Foundation.NSUrlSession session, Foundation.NSUrlSessionTask task, Foundation.NSError error) <0x101ffd2c0 + 0x00074> in <a7229a51752a4b23a2321c4878d035f4#b2bc9717d735d13fb9be7e6434b655da>:0 |
|
We are getting these too, out in the wild, a few per day. Sometimes an NRE, sometimes the below: System.ObjectDisposedException: The CancellationTokenSource has been disposed. |
|
Same here! |
|
I got these 2 lines while i was investigating a breakpoint - i believe the call timeout:
and
Not sure if these 2 lines help |
ghost
locked as resolved and limited conversation to collaborators
Steps to Reproduce
Unknown. It happened to me once while debugging, and we have several crash reports in App Center that look to be the same issue, but I haven't seen a consistent reproduction. This is in an iOS app running on iOS 13.
Expected Behavior
NSUrlSessionHandlerDelegate.SetResponsedoesn't crash the app with a NullReferenceExceptionActual Behavior
NSUrlSessionHandlerDelegate.SetResponsecrashes the app with a NullReferenceException.inflight.CancellationTokenSourcewas null on this line:Environment
The text was updated successfully, but these errors were encountered: