firebase-ios-sdk: [Firebase/Performance] `GULObjectSwizzler` crashes on `objc_disposeClassPair`.

[REQUIRED] Step 1: Describe your environment

  • Xcode version: 13.1
  • Firebase SDK version: 8.10
  • Installation method: Any
  • Firebase Component: Firebase
  • Target platform(s): iOS

[REQUIRED] Step 2: Describe the problem

Steps to reproduce:

When using Firebase Performance along with Detox Sync (a framework used by Detox), there is a double ISA Swizzling by the components. This causes a crash when NSZombiesEnabled=NO.

  • Firebase Performance does ISA Swizzling (using GULObjectSwizzler).
  • Detox Sync does ISA Swizzling, with a dynamic subclass of the generated class, using DTXSwizzlingHelper.
  • GULObjectSwizzler calls to dispose its own generated subclasses using objc_disposeClassPair().
  • Crash with the error message: objc_disposeClassPair: class 'fir_<some uuid>_<class name>' still has subclasses, including 'fir_<some uuid>_<class name>(__detox_sync_<class name>)'!

This issue seems to be related: https://github.com/google/GoogleUtilities/issues/15. Also, another explanation of a similar issue, with zombie objects: https://github.com/firebase/firebase-ios-sdk/issues/8321#issuecomment-900321941 (instead Zombie objects, we have DetoxSync objects).


This branch of DetoxSync reproduces the issue: https://github.com/asafkorem/DetoxSync/tree/feature/reproduce-detox-sync-crash: build and run ExampleApp/TimersTest.xcodeproj with NSZombiesEnabled=NO.

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Reactions: 18
  • Comments: 28 (23 by maintainers)

Commits related to this issue

Most upvoted comments

Sample size of 10 runs which - for this bug, in the react-native-firebase harness - is strongly statistically significant already. Looks great, thanks for the ping @asafkorem

Thanks @paulb777! We’ll replace the zombie flag with this one on Detox tests execution. FYI @mikehardy.

Thanks @asafkorem! GoogleUtilities 7.7.0 is now published to CocoaPods and SPM with your PR that disables class disposing when the GULGeneratedClassDisposeDisabled environment variable is set.

Really sorry I took everyone down the wrong path with my plural-zombies typo! Thanks @asafkorem for seeing that and clearing it up. Excited to have this mitigated, my CI is running now in react-native-firebase based off your PR there Asaf

Thanks @mikehardy! this workaround works for us, NSZombieEnabled=YES after adding this.

@shamilovtim Note that you should use SIMCTL_CHILD_NSZombieEnabled (..Zombie.. and not ..Zombies..). I checked this on our test-app where I reproduced the issues with firebase-perfomance and it solves the crashes I encountered with.

you have to pass environment variables very very carefully to get them through the simctl layer, examine this - it would probably work? I will try later (but maybe a few days) if you don’t

SIMCTL_CHILD_NSZombiesEnabled=1

from

https://github.com/invertase/react-native-firebase/blob/b9af132fd6c8b5fe0b917716f6638906ab2e1157/package.json#L42

This is tested+working way that I inject my AppCheck debug key in (and yes, this a key you’d normally keep private, we accept that it is public in this case as it is on a project configured as a no-cost thing for testing / demonstration purposes only)

Thanks @mikehardy, and sorry about the inconvenience around this thus far. Looks like we’re really getting somewhere now.

@paulb777 just to note this is of intense interest in react-native-firebase repo, our test harness is based entirely on Detox, and last time I did a survey of available e2e test tools, Detox still won, even with the horrific ios e2e crash frequency we’re taking now. Would ❤️ to see some sort of fix or remediation or something here 😃

Firebase performance relies a lot on Swizzling to measure performance out of the box. We are working on figuring out solutions outside of swizzling (or keep its dependency really low) to measure the performance. This is indeed tricky. So, we are definitely a few months out for a clean solution here. Until then the Zombie flags seems to be an agreeable solution (if that works). We will make sure to explore that option and add that to our FAQs as well.

@mikehardy yes, I thought about it too and already tried this approach. I wasn’t able to set this environment variable using xcodebuild command 😕