This is Xamarin's bug tracking system. For product support, please use the support links listed in your Xamarin Account.
Bug 50891 - NSUrlSession HttpClient propertyForKey crash
Summary: NSUrlSession HttpClient propertyForKey crash
Status: VERIFIED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Xamarin.iOS.dll (show other bugs)
Version: XI 10.4 (C9)
Hardware: Macintosh Mac OS
: Highest blocker
Target Milestone: (C9)
Assignee: Sebastien Pouliot
URL:
Depends on:
Blocks:
 
Reported: 2016-12-26 19:51 UTC by Tom Gilder
Modified: 2017-01-11 06:21 UTC (History)
3 users (show)

See Also:
Tags:
Is this bug a regression?: Yes
Last known good build: Xamarin.iOS 10.3.1.7


Attachments
Crash log with stacktrace (9.24 KB, text/plain)
2016-12-26 19:53 UTC, Tom Gilder
Details
System info (1.60 KB, text/plain)
2016-12-26 19:54 UTC, Tom Gilder
Details
Crash test case (5.84 KB, application/zip)
2016-12-27 17:55 UTC, Tom Gilder
Details

Description Tom Gilder 2016-12-26 19:51:48 UTC
Testing out Cycle 9 with my app and NSUrlSessionHandler is crashing the app on Xamarin.iOS 10.4.0.54:

"Unhandled Exception: Foundation.MonoTouchException: Objective-C exception thrown.  Name: NSInvalidArgumentException Reason: *** -propertyForKey: only defined for abstract class.  Define -[System_Net_Http_NSUrlSessionHandler_WrappedNSInputStream propertyForKey:]!"

It's fine with Xamarin.iOS 10.3.1.7. Only crashing when using the NSUrlSession. Managed and CFNetwork handlers don't crash. Only tested on iOS Simulator, with iOS 10.2.
Comment 1 Tom Gilder 2016-12-26 19:53:25 UTC
Created attachment 19001 [details]
Crash log with stacktrace
Comment 2 Tom Gilder 2016-12-26 19:54:51 UTC
Created attachment 19002 [details]
System info
Comment 3 Sebastien Pouliot 2016-12-27 16:35:43 UTC
@Tom I have a good idea what the issue is. However it passed (at least) our dev unit tests and part of QA ones (QA is not complete on C9) so I'd like to have more details so we can add this case to our test suite.

Can you share with us the code to reproduce the issue ? thanks


@Manuel, our internal subclass does not override all the required members, see https://developer.apple.com/reference/foundation/nsstream?language=objc
Comment 4 Tom Gilder 2016-12-27 17:55:25 UTC
Created attachment 19009 [details]
Crash test case

This is enough to crash it:

  var content = new StringContent("", Encoding.UTF8, "application/json");
  var client = new HttpClient();
  await client.PostAsync("https://www.xamarin.com/", content);


Test project attached
Comment 5 Manuel de la Peña 2017-01-04 17:35:28 UTC
Pr for with a fix can be found here: https://github.com/xamarin/xamarin-macios/pull/1435

Will comment once it lands.
Comment 6 Sebastien Pouliot 2017-01-05 21:46:52 UTC
master 4777b80784f714d205424a90f7493173d5210bf2
cycle9 07fab028becdd32ac1a1143989d8bcf08903a7f7
Comment 7 Shruti 2017-01-06 06:58:04 UTC
**************************
Reproduce Status:
**************************

1. As per comment0 I am successfully able to reproduce this issue with the mentioned version of Xamarin.iOS 10.4.0.54 at my end.

Getting Error: "NSUrlSessionHandlerCrash[2976:35957] Unhandled managed exception:
Objective-C exception thrown.  Name: NSInvalidArgumentException Reason: *** -propertyForKey: only defined for abstract class.  Define -[System_Net_Http_NSUrlSessionHandler_WrappedNSInputStream propertyForKey:]!"

=======Supplement Info=======
Application Output: https://gist.github.com/Shruti360/86e1cdd6fbbef84f0bc1da1f348647df
Envt Info and IDE Log: https://gist.github.com/shrutis360/f0edc766b1694976be6b2462479dd201

2. As per comment6, I tried to verify the issue on Cycle 9 and Master build of XI and observed that issue still persist.

=======Supplement Info=======
Build Output: https://gist.github.com/Shruti360/51daea457a703cf27ad8bf2076d1a384
Application Output: https://gist.github.com/Shruti360/cc04c46bfae421b968d24535ebe25bde

C9 Envt Info and IDE Logs: https://gist.github.com/shrutis360/98374027a2a6a2853742c777fcfd2c5d

Master Envt Info and IDE Logs: https://gist.github.com/shrutis360/0e5f4598e9d8586fe7fa3add2518f216

Hence, marking this issue as Reopened.

Thanks!
Comment 8 Sebastien Pouliot 2017-01-06 15:45:13 UTC
@Shruti nice catch. You have the linker enabled (in the simulator) which is different from the attachment (but @Manuel should have tried it on device too).

What happens is that the code that solve the issue is not used (by the app) so the linker removes it (but iOS itself wants to call into it). I'll fix this.
Comment 9 Sebastien Pouliot 2017-01-06 21:38:02 UTC
The PR (from comment #5) only called the "abstract" native implementation (that simply assert). What was needed is an override that returns false (and null) for both API.

It seems that using the PR with the test case that PostAsync hang (instead of asserting). That made it look like it was fine. 

I'm looking into why that happened since the build bots are still down. It's not linker  related, but maybe it's because the partial static registrar could be used on sim/dont link?
Comment 10 Sebastien Pouliot 2017-01-06 22:40:10 UTC
It seems to be a debugger/UI "issue". The debugger execution is stopped (like a breakpoint). If resumed then we get the abort in the application output.

However since

* the app screen is black;
* the editor does not change (file / source location);
* the call stack pad is empty; and
* we don't get any dialog (or other visual clue) that the process is stopped

it's easy to think the execution has completed successfully (and kill the app).

The behaviour seems to happen only with a prebuilt simlauncher, i.e. if linking or giving mtouch other options (e.g. `--registrar:static`) then the abort happens without a break.
Comment 11 Sebastien Pouliot 2017-01-06 22:47:08 UTC
PR cycle9 https://github.com/xamarin/xamarin-macios/pull/1451
Comment 13 Shruti 2017-01-09 08:25:55 UTC
I have checked this issue following by comment9 and 10 with the Cycle9 build mentioned in comment12 and observed that the issue is working fine now.

*OK Tested on iOS Simulator, with iOS 10.2, 10.0 and iPhone 7 (physical device) with iOS 10.2.

Supplement Info: 
Build Output: https://gist.github.com/Shruti360/c0f8cbf0face96f362f8ce9a6ce2401a
Application Output: https://gist.github.com/Shruti360/9ed7ca9803934971251f96c82a9fa8c9

Envt Info and IDE Logs: https://gist.github.com/shrutis360/340a837df58b358bb90f9f8bce9b355e

I will close this issue once the fix is verified on Master as well.

Thanks!
Comment 14 Shruti 2017-01-10 08:34:24 UTC
@Sebastien, Please commit the fix on Master.
Comment 16 Shruti 2017-01-11 06:21:27 UTC
I have checked this issue with the latest build of Master XI and observed that this issue is fixed now.

=======Supplement Info=======
Build Output: https://gist.github.com/Shruti360/99cc9550b5ee7bb276c7eb6f9cee8f74
Application Output: https://gist.github.com/Shruti360/30865e66902909a9e164884404a9199b

Envt and IDE Logs: https://gist.github.com/shrutis360/ad7a44853cdd02f4d71173f0f17b8f8b

Hence, marking this issue as verified fixed.

Thanks!

Note You need to log in before you can comment on or make changes to this bug.