Xamarin-macios: Reproducible native crash bug with WKWebView in latest Xamarin.iOS

[nejdu]

(Coming from Bugzilla we were at a loss where to post this and wrongly submitted this bug report to the Xamarin forums first, sorry about that).

Loading a WKWebView with a HTML string twice using a WKNavigationDelegate that overrides DecidePolicy crashes Xamarin.

Steps to Reproduce

Create a new project with this UIVIewController

    public class TestController : UIViewController
    {
        class Navigator : WKNavigationDelegate
        {
            public override void DecidePolicy(WKWebView webView, WKNavigationAction navigationAction, Action<WKNavigationActionPolicy> decisionHandler)
            {
        decisionHandler(WKNavigationActionPolicy.Cancel);
        }
        }

        WKWebView page;

        void AddPage()
        {
            page?.RemoveFromSuperview();
            page = new WKWebView(new CGRect(0, 0, 400, 400), new WKWebViewConfiguration());
            page.NavigationDelegate = new Navigator();
            page.LoadHtmlString("", new NSUrl(NSBundle.MainBundle.ResourceUrl + ""));

            View.AddSubview(page);

            Console.WriteLine("OK");
        }

        public override void ViewDidLoad()
        {
            UIButton button = new UIButton(new CGRect(200, 500, 0, 0));

            button.SetTitle("Add page", UIControlState.Normal);
            button.SizeToFit();
            button.TouchUpInside += (sender, e) => AddPage();

            View.AddSubview(button);
        }
    }

Pressing the button should add an empty webview to the view controller and print "OK" to stdout. This works the first time, but waiting a second and pressing the button again will add the page, print "OK" and then either simply hang or crash the application Sometimes you might get away with more presses, but ultimately it does crash.

Expected Behavior

App shouldn't crash

Actual Behavior

App crash

[spouliot]

From a quick look you're never calling decisionHandler when you override DecidePolicy and that goes against Apple API documentation.

Your delegate can either call the block immediately or save the block and call it asynchronously at a later time.
https://developer.apple.com/documentation/webkit/wknavigationdelegate/1455641-webview?language=objc

[nejdu]

That is true, I've updated the test case to highlight that's not the issue.

[nejdu]

We've concluded that we can work around the crash if we never remove the view from its superview, so if you're having random webview related crashes in your apps (which it seems quite a few have been reporting on) you might want to try that.

[VincentDondain]

Hi, here is a sample app based on the code snippet you sent us https://www.dropbox.com/s/gn2ici9dmzosaot/Issue4130.zip?dl=0

When pressing Add multiple times (quickly) you'll likely get: May 29 17:13:23 iMac-5K-MSFT com.apple.CoreSimulator.SimDevice.981F2B61-3230-48B1-BFCA-603D6D348800[10462] (UIKitApplication:com.xamarin.Issue4130[0xc31][10487][10560]): Service exited due to signal: Trace/BPT trap: 5 sent by exc handler[0] in the console output.

I believe what's happening is that the delegate is being called after the WebView is removed from its superview and that's causing the native crash. Not having page?.RemoveFromSuperview(); is indeed a way to fix this.

[nejdu]

Maybe, probably? It does crash even with the decisionHandler delegate set to null, though. Not overriding DecidePolicy makes the issue go away completely, so it's most definitely something to do with the delegate.

[VincentDondain]

I also see the message bellow in the console:

May 30 09:26:34 Vincents-MacBook-Pro Issue4130[34948]: objc[34948]: Class VCWeakObjectHolder is implemented in both /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/Library/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot/System/Library/PrivateFrameworks/AVConference.framework/Frameworks/ViceroyTrace.framework/ViceroyTrace (0x124acc4d0) and /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/Library/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot/System/Library/PrivateFrameworks/AVConference.framework/AVConference (0x12494ce38). One of the two will be used. Which one is undefined.
May 30 09:26:35 Vincents-MacBook-Pro Issue4130[34948]: assertion failed: 17E199 15E217: libxpc.dylib + 71589 [22B51B1C-1E07-3FEE-A4E9-60E90E7520D6]: 0x7d
May 30 09:26:42 Vincents-MacBook-Pro Issue4130[34948]: BUG in libdispatch client: kevent[mach_recv] monitored resource vanished before the source cancel handler was invoked
May 30 09:26:42 Vincents-MacBook-Pro launchd_sim[34838]: BUG in libdispatch client: kevent[mach_recv] monitored resource vanished before the source cancel handler was invoked

Tested with the following environment: https://gist.github.com/VincentDondain/348fd16831717d2a24fe200c2938e72c

[VincentDondain]

Full console output: https://gist.github.com/VincentDondain/ea348f858ce394c34a34a03965766229

[VincentDondain]

Issue4130_2018-05-29-174719_iMac-5K-MSFT.crash.zip

[rolfbjarne]

This is the crashing thread:

Thread 5 Crashed:: Finalizer
0   com.apple.WebKit                0x00000001153e66aa non-virtual thunk to WebKit::WebCookieManagerProxy::processPoolDestroyed() + 0
1   ???                             0xffff9f7fffed83a0 0 + 18446637970836259744
2   com.apple.WebKit                0x00000001154f718f WebKit::WebProcessPool::~WebProcessPool() + 247
3   com.apple.WebKit                0x00000001155b5ec2 -[WKProcessPool dealloc] + 27
4   libobjc.A.dylib                 0x0000000118423a6e objc_object::sidetable_release(bool) + 202
5   com.apple.WebKit                0x000000011559ee87 -[WKObject dealloc] + 25
6   libobjc.A.dylib                 0x0000000118423a6e objc_object::sidetable_release(bool) + 202
7   com.apple.WebKit                0x000000011547cda3 WebKit::WebPageProxy::~WebPageProxy() + 2303
8   com.apple.WebKit                0x000000011559ee87 -[WKObject dealloc] + 25
9   libobjc.A.dylib                 0x0000000118423a6e objc_object::sidetable_release(bool) + 202
10  com.apple.WebKit                0x000000011526630d API::FrameInfo::~FrameInfo() + 45
11  com.apple.WebKit                0x0000000115597da3 -[WKFrameInfo dealloc] + 27
12  libobjc.A.dylib                 0x0000000118423a6e objc_object::sidetable_release(bool) + 202
13  com.apple.WebKit                0x000000011539bd73 API::NavigationAction::~NavigationAction() + 183
14  com.apple.WebKit                0x000000011559d585 -[WKNavigationAction dealloc] + 27
15  libobjc.A.dylib                 0x0000000118423a6e objc_object::sidetable_release(bool) + 202
16  com.apple.WebKit                0x00000001152d6b8c WTF::BlockPtr<void (WKNavigationActionPolicy)> WTF::BlockPtr<void (WKNavigationActionPolicy)>::fromCallable<WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction(WebKit::WebPageProxy&, WTF::Ref<API::NavigationAction, WTF::DumbPtrTraits<API::NavigationAction> >&&, WTF::Ref<WebKit::WebFramePolicyListenerProxy, WTF::DumbPtrTraits<WebKit::WebFramePolicyListenerProxy> >&&, API::Object*)::$_2>(WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction(WebKit::WebPageProxy&, WTF::Ref<API::NavigationAction, WTF::DumbPtrTraits<API::NavigationAction> >&&, WTF::Ref<WebKit::WebFramePolicyListenerProxy, WTF::DumbPtrTraits<WebKit::WebFramePolicyListenerProxy> >&&, API::Object*)::$_2)::'lambda'(void const*)::__invoke(void const*) + 82
17  libsystem_blocks.dylib          0x0000000118e4d98a _Block_release + 111
18  com.xamarin.Issue4130           0x0000000109d67d66 reference_queue_proccess_all + 134
19  com.xamarin.Issue4130           0x0000000109d67a2d finalizer_thread + 781
20  com.xamarin.Issue4130           0x0000000109e160a0 start_wrapper + 704
21  libsystem_pthread.dylib         0x000000011935b661 _pthread_body + 340
22  libsystem_pthread.dylib         0x000000011935b50d _pthread_start + 377
23  libsystem_pthread.dylib         0x000000011935abf9 thread_start + 13

I wonder if this is crashing because we're releasing a block on a different thread.

[rolfbjarne]

I wonder if this is crashing because we're releasing a block on a different thread.

This seems to be correct, I can't reproduce the crash anymore if I force blocks to be released on the main thread.

Unfortunately this will be a bit complicated to fix.

[rolfbjarne]

A small overview of what happens:

1. When an API called by native code takes a block as an input parameter, we'll create a delegate for that block when the method is invoked, and retain the block until the delegate has been collected by the GC.
2. When the GC runs, and collects the delegate, we'll release the corresponding block. This will be done on the finalizer thread (as can be seen in the stack trace above: https://github.com/xamarin/xamarin-macios/issues/4130#issuecomment-393586239)
3. If releasing the block ends up executing other code, and that other code has specific threading requirements, bad things happen (i.e. this bug).

I'm guessing that this was introduced by 2fe44b98900b2b2a52f699b5d3ed1980387c1bb3, which fixed a memory leak (blocks were accidentally retained twice, thus they would never be deallocated, and "the other code with threading requirements" from point 3 above would never execute).

Options:

1. Always release blocks on the main thread.
   
   
   
   • Con: this may run into the reverse case (we end up executing code on the main thread that should never be executed on the main thread).
   
   
   • Con: it's slower, and will cause an unnecessary performance degradation for API that work fine with the current behaviour.
   
   
   • Pro: it's fairly simple to implement.
   
   
2. Try to release on the same thread where we saw the block the first time.
   
   
   
   • Con: there's no generic way to execute something "on the same thread", and we run into a bunch of corner cases:
     
     
     
     
     
     • We can store the current run loop or dispatch queue and try to schedule something there when we're releasing the block, but:
       
       
       
       
       
       
       
       • The stored queue might be a queue that runs on multiple threads, so "execute on the same thread" won't be what we accomplish.
       
       
       
       
       • The stored queue might have stopped or not even exist anymore, thus scheduling something there won't do anything.
       
       
       
       
       • The stored queue might be doing a long-running task, which might never even complete.
       
       
       
       
     
     
     
   
   
   • Con: it's slower, and will cause an unnecessary performance degradation for API that work fine with the current behaviour.
   
   
   • Pro: It seems more logical to try to release blocks on the same thread (than always releasing on the main thread).
   
   
3. Add support for specifying how a block should be freed in our bindings (either on main thread or in the finalizer thread).
   
   
   
   • Con: most complicated to implement, since it requires changes to the generator, the registrar and the runtime.
   
   
   • Pro: it's the most flexible, and less likely to cause regressions (since we'll only change the behaviour for API that's known to require the non-default behaviour).
   
   
   • Pro: anything we do will be slower than the current behaviour; this will limit any performance degradation to the API that actually needs a different behaviour.
   
   

@spouliot I'm leaning towards option 3, but do you have any other ideas/opinions?

[Karthikeyan-tv]

@rolfbjarne , I too got the same app crash in my (Xamarin.Forms) iOS mobile application. When can I expect the fix for this?

Here is my crash report,
https://gist.github.com/Karthikeyan-tv/dfce2ed1f4854a8f9c90b40a6cc33dda

[rolfbjarne]

After discussing with Sebastien, I've implemented option 1 (in PR #4312).

[rolfbjarne]

@nejdu here's a Xamarin.IOS package with the fix: xamarin.ios-11.13.0.19.pkg, can you try that and see if it works for you?

[dkarzon]

@rolfbjarne I had this same issue. Took out the block that overrode the DecidePolicy and it started working (to confirm it was happening there).
I then put that method back in and applied your Xamarin.IOS package and it works, thanks!

[nejdu]

@rolfbjarne Nice. I'm out of office and we've already shipped with a workaround, feel free to close if it's working for @dkarzon

[rolfbjarne]

@nejdu it's a bug on our side, so we'll close this once we've fixed that bug.

[RWardw]

Hey @rolfbjarne thank you for addressing this. We were experiencing the same issue and your package seems to have fixed the problem we were having. Approximately, when can we expect the fix to be distributed through normal channels? Thanks!

[rolfbjarne]

@RWardw thanks for confirming the fix. Currently this will most likely be shipped in our d15-9 release, which is scheduled sometime late this fall.

[mrdaneeyul]

Like @Karthikeyan-tv, I'm having trouble with this in a Xamarin.Forms iOS app. Will this fix be available to Xamarin.Forms with the dl15-9 release as well? Is there a temporary workaround?

EDIT: Looks like the workaround is to install xamarin.ios-11.13.0.19.pkg (from above) on the Mac build machine!

[Depechie]

@rolfbjarne indeed installing the xamarin.ios-11.13.0.19.pkg also fixes our wkwebview problem in xamarin forms.
Question though, we are on schedule to release an update for our app in mid August. This pkg does it 'only' contain a fix for this problem, or is this already a pkg with code for other problems? Because if it is the latter, we are not very confident for using the pkg for our app release. Meaning we won't be able to release...

[kalyanmungi]

This issue is fixed in Xamarin.iOS 11.13.0.19 but this is not included in 11.14(current stable). I still see these crashes in 11.14

[Depechie]

@kalyanmungi that is true as stated here https://github.com/xamarin/xamarin-macios/pull/4500#issuecomment-409839221 you still need to install the fix as mentioned.

[rolfbjarne]

This is a package of the latest stable (Xamarin.iOS 12.0) + this fix: xamarin.ios-12.0.0.16

[Macjon]

@rolfbjarne is there a release date for this fix? And I have the same question as @Depechie, does this pkg contains only this fix or does it also contains other fixes?

[rolfbjarne]

@Macjon probably somewhere mid/late November. The package in my previous comment contains only this fix.

[mariorasch]

As I am still experiencing crashes in the Finalizer thread using iOS 12.1.0.15 I'd like to ask if this bug should be resolved in this version?

[Macjon]

@mariorasch as @rolfbjarne already mentioned, the fix will be released mid/late November (probably version iOS 12.0.0.16).

[rolfbjarne]

probably version Xamarin.iOS 12.0.0.16

Not quite, Apple released Xcode 10.1 so we had to make an interim release, which turned out to be Xamarin.iOS 12.1. This means that the release with this fix will be Xamarin.iOS 12.2 (which is currently in the alpha+beta channels).

The date has also slipped slightly: it's scheduled at the same time as Connect, which is the first week of December.

[zhekin]

@rolfbjarne, could you please provide a standalone xamarin.ios-12.1.0.16 package with the fix?

[ivancitin]

Hi all, i'm facing this issue in a Xamarin.Mac app, the fix in Xamarin.iOS will also fix this for my Xamarin.Mac app? Thanks

[VincentDondain]

Hi,

@zhekin so I think what you want is 12.2.1.10 (I think 12.1.0.16 wouldn't have the fix because it's based on a previous release branch).

Note that those links are accessible by everyone.

To get it simply go to the desired release branch on GitHub: https://github.com/xamarin/xamarin-macios/commits/d15-9 (you kinda have to follow where we're at release-wise, next one is d15-9) and click on the green checkmark or red arrow (it doesn't matter) and you'll see packages to download there.

Important to note that these can be unreleased packages (a.k.a not approved by QA) so it's at your own "risks".
This particular package I believe is our next d15-9 preview coming up shortly (will replace the packages in the alpha / beta channels). It adds the Xcode 10.1 support in comparison to 12.2.1.9 (currently released).

[VincentDondain]

@ivancitin given the nature of the change I believe it should also be fixed for Xamarin.Mac so please give 5.2.1.10 a try. Feel free to report back here if you have any issue (:

[ivancitin]

@VincentDondain Thanks for circling it back, i did try 5.2.1.10 alone but seems to brake other parts of my dev environment, app didn't start at all with some binding failures, i may have to get other components updated for they to play along. I can live with the issue until this gets released.

[rolfbjarne]

@zhekin As my colleague Vincent mentioned, please use this package: xamarin.ios-12.2.1.10.pkg, it's from the upcoming d15-9 release. It's not in stable yet, but most probably this is the package that will end up in stable next month.

[VincentDondain]

@ivancitin feel free to open a new issue with repro steps, detailed build logs, crash report and version information and we'll be happy to help solve those issues (:

[alicint]

this bug was in my app as well, found the reason after 2 years of looking. good job xamarin!

[alicint]

@zhekin As my colleague Vincent mentioned, please use this package: xamarin.ios-12.2.1.10.pkg, it's from the upcoming d15-9 release. It's not in stable yet, but most probably this is the package that will end up in stable next month.

any idea how to install this Xamarin.IOS version on windows? I cannot connect to my mac build server after installing that on my mac.

[josegallardo]

Hi @alicint, what issues are you getting trying to connecto to your Mac from Windows?
Is that trying to Pair to Mac from Visual Studio? What errors do you get?

If VS finds out an incompatible x.iOS on the Mac, it will try to download and install a compatible one, unless you have an older VS version in Windows.

To connect with the version @VincentDondain is suggeting, you should use Visual Studio Preview (15.9).
You can install it from:
https://visualstudio.microsoft.com/vs/preview/

[VincentDondain]

Hi,

I provided an unreleased package, since then 12.2.1.10 has been approved and released to the official beta channel.

Please use the installer as suggested above to get the updated Xamarin.iOS (:

[kalyanmungi]

@rolfbjarne I tried installing your package for the WKWebView crash, it fixed in one place but it's crashing in another place with different WKWebView. Please look at the crash log below

MAIN THREAD - CRASHED CoreFoundation CFRelease WebKit API::Data::~Data() WebKit -[WKNSData dealloc] WebKit -[WKWebView loadData:MIMEType:characterEncodingName:baseURL:] MobileXiOS - plcrash::MS::async::dwarf_cfa_state_iterator<unsigned long long, long long>::next(unsigned int*, plcrash::MS::async::plcrash_dwarf_cfa_reg_rule_t*, unsigned long long*) + 598068 MobileXiOS - plcrash::MS::async::dwarf_cfa_state_iterator<unsigned long long, long long>::next(unsigned int*, plcrash::MS::async::plcrash_dwarf_cfa_reg_rule_t*, unsigned long long*) + 596848 MobileXiOS - plcrash::MS::async::dwarf_cfa_state_iterator<unsigned long long, long long>::next(unsigned int*, plcrash::MS::async::plcrash_dwarf_cfa_reg_rule_t*, unsigned long long*) + 19876 MobileXiOS - plcrash::MS::async::dwarf_cfa_state_iterator<unsigned long long, long long>::next(unsigned int*, plcrash::MS::async::plcrash_dwarf_cfa_reg_rule_t*, unsigned long long*) + 19556 MobileXiOS - plcrash::MS::async::dwarf_cfa_state_iterator<unsigned long long, long long>::next(unsigned int*, plcrash::MS::async::plcrash_dwarf_cfa_reg_rule_t*, unsigned long long*) + 19324 libdispatch.dylib _dispatch_client_callout + 12 MobileXiOS xamarin_release_block_on_main_thread

[Depechie]

For reference, in Visual Studio for Mac with Xamarin.iOS v 12.2.1.10 release version we got today, everything seems fixed!
Great!!

[kalyanmungi]

@rolfbjarne @VincentDondain https://github.com/xamarin/xamarin-macios/issues/4130#issuecomment-437892559

Please look at the above crash log, it is crashing exactly after running "xamarin_release_block_on_main_thread" method, which is added to fix this native wkwebview crash.

[VincentDondain]

Hi,

@kalyanmungi we're missing information to investigate your (different) issue.

Given that others reported that this issue is fixed (in their case) and you reported "it's fixed in one place but it's crashing in another", I suggest you create a new issue so we can address your _specific_ new problem not previously covered here.

This issue is closed and getting pretty long. It'll just add confusion IMHO.

---

In the new issue please provide your full build logs, crash reports, test case (very important for us to reproduce) and all your version information.

To get full build logs just set the log verbosity to diagnostic at the following locations:

• On Visual Studio for Mac: Preferences > Projects > Build

• On Visual Studio for Windows: Tools > Options > Projects and Solutions > Build and Run

  On Visual Studio for Windows you also want to add -v -v -v -v to the mtouch additional arguments by right-clicking the project in the solution explorer and selecting Properties.

  Note: this is done automatically on Visual Studio for Mac when the log verbosity is set to diagnostic.

  The easiest way to get exact version information:

• On Visual Studio for Mac: "Visual Studio" menu, "About Visual Studio" item, "Show Details" button.

• On Visual Studio for Windows: "Help menu", "About Microsoft Visual Studio" item.

  Then copy/paste the version information (you can use the "Copy Information" button).