pendo-io / pendo-mobile-sdk

Pendo captures product usage data, gathers user feedback, and lets you communicate in-app to onboard, educate, and guide users to value
https://www.pendo.io
Other
58 stars 2 forks source link

Pendo Causes Crashes on iOS 13+ If Deployment Target Is Set To iOS < 13 #31

Closed adiracu closed 1 year ago

adiracu commented 1 year ago

Hi.

We've recently experienced quite a large share of insta-crashes in production, after linking Pendo (see attached crash report).

The root issue was that Pendo made the entire app prefer using the embedded Swift libraries instead of the system one.

Our app had the deployment target set to iOS 11, so for iOS 11 & 12, Xcode would embed the Swift libraries within the app.

Normally, on iOS 11 & 12, iOS would use the embedded Swift libraries whereas on iOS 13+ it would use the system Swift library, from /usr/lib/swift.

Pendo has its load commands specified in the wrong order, prefering @executable_path/Frameworks first, instead of /usr/lib/swift. You can check this by running otool -l on the Pendo binary. Normally, you'd want /usr/lib/swift to be the first one.

Because Pendo had those commands in a different order, for some users on iOS 13+, it would try to load the embedded Swift libraries & insta-crash, because those libraries were meant to be ran on iOS 11/12, not on 13+.

In our case, this affected almost all updating users, but it didn't affect any new installs. I'm guessing that for new installs on iOS 13+, the App Store is smart enough to take care of this particular edge case and strip the embedded Swift libraries altogether.

We did fix it by dropping support for iOS 11/12, so there are no more embedded Swift libraries to try and open now, but I'm guessing this affects all clients that integrate with Pendo and have a deployment target < 13 set.

otool -l Pendo.xcframework/ios-arm64_armv7/Pendo.framework/Pendo:

...
Load command 45
          cmd LC_LOAD_WEAK_DYLIB
      cmdsize 48
         name @rpath/libswiftos.dylib (offset 24)
   time stamp 2 Thu Jan  1 02:00:02 1970
      current version 1023.0.0
compatibility version 1.0.0
Load command 46
          cmd LC_RPATH
      cmdsize 40
         path @executable_path/Frameworks (offset 12)
Load command 47
          cmd LC_RPATH
      cmdsize 40
         path @loader_path/Frameworks (offset 12)
Load command 48
          cmd LC_RPATH
      cmdsize 32
         path /usr/lib/swift/ (offset 12)

report.crash.log

MikePendo commented 1 year ago

Hi @adiracu U r right we had specified the order of the @rpath not in the default Xcode way. There are several ways currently to resolve the issue.

  1. As you have already mentioned you can set the deployment target to iOS 12.2 (Not IOS 13)
  2. Use App thinning, (When distributing the app via test flight or app store the process is done by apple, while distributing by any other tool may not have the same results). Also please note the app thinning is MUST according to apple when u distributing.
    However, if you distribute an in-house enterprise app, or distribute your builds to testers without using TestFlight, you must enable app thinning when you export your app

    Apple docs

I hope this week we will have a new release which will resolve the issue (either we will exclude swift or we will change the path) Sorry for the inconvenience, I willl close the issue as I have nothing more to add if u have any other questions regarding the issue please feel free to write us here

adiracu commented 1 year ago

The fix should be to just change the order of the paths via the LD_RUNPATH_SEARCH_PATHS build setting, with /usr/lib/swift being the first one. Theoretically, excluding it should work as well as the Xcode (or the App Store?) will add it when required, but I wouldn't personally trust them :)

Thanks for the prompt response!

MikePendo commented 1 year ago

@adiracu We have just released a fix (we excluded the Swift libs now from the main SDK). We will only have them in the swiftui binary (I will fix the order for that binary soon, in any case that binary is fr iOS 13 and up s should not have above issue)