Open danniss10 opened 2 years ago
Hi @danniss10
This issue happens because OneSignal's swizzling implementation uses method_exchangeImplementations
which will make the actual original implementation inaccessible to any other swizzling agent, like Instabug; as the only way to get it is to guess the new selector added by the swizzling agent before us which is also inaccessible.
Instead, they should use a different approach where they change the implementation of willTerminate...
method without changing the _cmd
, using something like method_setImplementation
This screenshot shows the final situation after OneSignal swizzles and Instabug swizzles
This screenshot shows how it should look like
References:
An update, I have tested Instabug with OneSignal swizzling code, and I couldn't reproduce this issue.
I have tested with the following project if you wanna give it a shot: InstabugOneSignal.zip
Thanks for checking and for the helpful explanation @yousefhamza!
Did more digging and was actually able to narrow it down to a commit where we integrated Google Firebase into our project.
I don't understand why Firebase doesn't show up in the crash logs, but it looks like Firebase also does some swizzling which may interfere.
We were able to get the crash to stop by adding this flag to our info.plist: FirebaseAppDelegateProxyEnabled=NO. Per this post on stack overflow, it disables the swizzling.
For reference, we use firebase-ios-sdk 9.6.0 and import FirebaseRemoteConfig and FirebaseAnalytics
Worth noting that OneSignal recently patched a similar crash in v3.11.2 https://github.com/OneSignal/OneSignal-iOS-SDK/pull/1100
I was able to reproduce this crash in this version of the sample project: InstabugOneSignal.zip
Explanation of the crash:
applicationWillTerminate:
, so OneSignal adds its own as the implementation of applicationWillTerminate
in your AppDelegate classapplicationWillTerminate:
implementation there, we do not swizzle if it's not there, so we update the implementation to our implementation + OneSignal's implementationapplicationWillTerminate
is not their implementation, so they add their own implementation to oneSignalApplicationWillTerminate:
and swipe the implementations of applicationWillTerminate:
and oneSignalApplicationWillTerminate:
Which will end up in exactly the setup shown in the first screenshot here: https://github.com/Instabug/Instabug-SP/issues/23#issuecomment-1295510305
This scenario was happening in OneSignal via changing the app delegate; setting it to nil
then re-set it to the original class again, so you may look into similar scenarios in your code as well
@danniss10 Thanks for the extra detail on Firebase also being included and this detail:
We were able to get the crash to stop by adding this flag to our info.plist: FirebaseAppDelegateProxyEnabled=NO. Per this post on stack overflow, it disables the swizzling.
@danniss10 Just to confirm what it takes to reproduce the issue?:
FirebaseRemoteConfig
and FirebaseAnalytics
with Swizzling left on (default)
FirebaseAppDelegateProxyEnabled=NO
is NOT present.@yousefhamza I am one of the devs who work on the OneSignal iOS SDK. Great diagram showing the selector to implementation mapping of how swizzling is effecting things here! It's accurate from the OneSignal side of things.
OneSignal does use method_exchangeImplementations which does change the _cmd
(AKA selector), so it does leave a trace behind. While that is a pro I am not convinced that using method_setImplementation would cause less conflicts with others swizzlers.
In your second test project (InstabugOneSignal.zip) you copied in some of the OneSignal logic but you're missing a key piece where OneSignal ensures it never swizzle the same class more than once: https://github.com/OneSignal/OneSignal-iOS-SDK/blob/3.11.2/iOS_SDK/OneSignalSDK/Source/UIApplicationDelegate%2BOneSignal.m#L78
I think that is why in your first project (InstabugOneSignal.zip) you were not able to reproduce the bug since you were using the full OneSignal SDK.
@yousefhamza Really appreciate the deep dive here on the swizzling investigation with OneSignal!
Thanks for the kind words 🙏
About this
OneSignal does use method_exchangeImplementations which does change the _cmd (AKA selector), so it does leave a trace behind. While that is a pro I am not convinced that using method_setImplementation would cause less conflicts with other swizzlers.
For the current bug, if the same sequence of events still happens, using method_setImplementation OneSignal implementation will be called, then Instabug implementaiton called, then OneSignal implementation called again, and that's it.
This is far from ideal as I guess this may result in inaccurate data on your side, but arguably better than an infinite loop
If anyone is still experiencing this, you might want to disable the Instabug crash report. I disabled it in my project because it also have Crashlytics for crash report so it's sort of convenient for us while we wait a definitive solution for this.
Hi @henrique-eguchi
That works because of the method in a conflict between us and OneSignal willTerminate
is used in one of the features on our crash reporting called "User terminations", you can know more about it here: https://docs.instabug.com/docs/ios-reporting-crashes#user-terminations
If you still wish to use our crash reporting product, you can reach out to support to disable this feature until OneSignal can work out a solution with us 🙏
Steps to Reproduce the Problem
Actual Behavior
Crash occurs with some sort of infinite loop, see crash log below.
Instabug integration code
SDK Version
Instabug 11.4.1
iOS Version
Observed in iOS 16, 15, 14 crash reports
Device Model
Multiple
Crash log