react-native-share-menu icon indicating copy to clipboard operation
react-native-share-menu copied to clipboard

Out of memory crashes are more frequent on iOS 17 when sharing certain URLs

Open freethejazz opened this issue 2 years ago • 6 comments

Note: This has been discussed in #256, but adding as a separate issue for visibility and potential fixes.

Summary It seems as if the behavior of the default SLComposeServiceViewController has changed in iOS 17 in a way that dramatically increases the amount of memory used within a share extension. We are using a custom iOS share view, but the problematic SLComposeServiceViewController is still being loaded behind it. We have implemented a fix via patch-package, but in general feels like there's a better, more universal way of dealing with this. I don't know if there's a way to configure the new iOS behavior to not take up so much extra memory, it might be helpful to change the behavior of this library to load up the bare minimum default UI (if at all) when using a custom iOS share view. It wouldn't solve the memory use issue for anyone relying on the default UI, but it does mitigate the impact for those who had already opted into custom share views.

Detail We recently began receiving reports more frequently about our share extension crashing, and we were pretty sure our extension was being jettisoned due to memory consumption. These reports were for the same version of the app, but we started to associate the increase in crashes with iOS 17.

Through troubleshooting and the very helpful comments from @RDR96 and @jb4e in #256, we (@rpc1910 and I) were able to confirm that the crashes were happening more frequently in iOS 17, are dependent on the specific content being shared, and were able to implement a fix.

It seems that iOS 17 eagerly loads websites of the URLs that are shared, and that all of the associated resources (images/css/javascript) are being loaded as well. All of the memory required to load these resources is being associated with the share extension process, so if the page is heavy with resources, the extension will crash.

We implemented a fix per the recommendation in this comment, and our memory consumption went from 120MB+ down to 20MB on iOS 17.1.1. Meanwhile, the memory consumption remained unchanged at ~55MB on iOS 16.6.1.

Edit: Confirming this behavior also persists on 17.1.2

freethejazz avatar Dec 01 '23 02:12 freethejazz

@freethejazz I just wanted to clarify a detail in what you said:

The comment you linked to recommended replacing SLComposeServiceViewController with SLComposeViewController, and you said you followed up on that fix. But the next paragraph says "the problematic SLComposeServiceViewController is still being loaded behind it.", and I'm having trouble understanding how that could happen if it was replaced with SLComposeViewController. Could you clarify?

lindboe avatar Dec 05 '23 18:12 lindboe

Hey @lindboe! First of all, thanks to you, the Infinite Red, and the Expensify team for adopting this project and helping keep the original great work evolving and useful 🙏. I first used Ignite for a project back when it was 1.x and was comforted to see that this project has a new home with you all.

I think I understand the confusion, probably due to me jumping around a bit. That paragraph could be nearly taken as a separate statement to the timeline/details above it, which is

  • a summary of my understanding of the root cause (default SLComposeServiceViewController, with it's own and changing behavior, loading up quietly in the background when we're not using it)
  • a comment loosely proposing the usefulness of a way of disabling it in the library

I've also edited the original text. Does that help clarify?

freethejazz avatar Dec 07 '23 23:12 freethejazz

Thanks for the kind words! And yes, that does help clarify.

I'm hoping we might actually be able to incidentally fix this as we add a third view controller option in https://github.com/Expensify/react-native-share-menu/issues/285. Since that view controller needs to extend UIViewController, not any SLCompose controller, we'll need to extract and share more logic with ShareViewController. This should make it easier to make ReactShareViewController not extend ShareViewController anymore, and can instead extend whatever class we'd like.

That said, I want to try to be conscious of backwards compatibility where we can. It sounds like using SLComposeViewController instead was a pretty straightforward change, just swapping out the classes? I'm not particularly familiar with the SLCompose view controllers, so I don't know if there's a particular reason we'd want to use them for the custom RN view as opposed to, say, UIViewController (I did find one native blog post from 2015 (!) supporting use of UIViewController: https://catthoughts.ghost.io/extensions-in-ios8-custom-views/)

In the tracking ticket https://github.com/Expensify/react-native-share-menu/issues/294, I mentioned we're not currently prioritizing any work for the share extension UIs, but if we happen to be doing work that will fix this we'll happily include it. If it turns out it's not fixed after #285, we'd be happy to discuss other options, and if you'd like to test work as it comes in that could be helpful! (This may not be very soon, with our current schedule).

lindboe avatar Dec 08 '23 00:12 lindboe

I've been struggling with this for almost a year.. It's not a iOS 17 issue, I have an iPhone 8 with iOS 16 and it's the same.

This is what fixes this issue (in the ShareViewController):

+    override func loadPreviewView() -> UIView! {
+        return nil
+    }

This stops the SLComposeServiceViewController from generating the preview (as otherwise it loads the preview using webview, even tries to play the videos from the page)

mstrokin avatar Dec 11 '23 14:12 mstrokin

Thanks @mstrokin! We can confirm that this fix works for us as well.

freethejazz avatar Jan 15 '24 00:01 freethejazz

I got the error EXC_RESOURCE RESOURCE_TYPE_MEMORY (limit=120 MB, unused=0x0) when sharing heavy files, do you know why?

cuonglt-slz avatar Jan 15 '24 13:01 cuonglt-slz