muxinc / mux-stats-sdk-avplayer

Mux integration with `AVPlayer` for iOS Native Applications
Apache License 2.0
45 stars 21 forks source link

Crash when using SPM and launching on Mac Catalyst #203

Closed andrewjl-mux closed 1 year ago

andrewjl-mux commented 1 year ago

Hey @gabors, I've created this issue in reference to the crash you reported on Mac Catalyst when installing the SDK using SPM as a dependency to a library target.

I tested an example app on Mac Catalyst with v3.3.2 of the SDK using that installation method without issues.

I suspect the crash happens because the SDK dylib is incorrectly loaded at app launch and not due to a code signing issue. See this Apple DTS post (https://developer.apple.com/forums/thread/695481?answerId=696445022#696445022) about a similar sounding report. That lines up with the crash log you shared with a previous SDK version listing dyld (the process responsible for loading and linking dynamic libraries at runtime) errors in addition to the code signing message. This all makes me curious if the change removing the LC_RPATH warnings also fixed that crash.

If you get a chance to repeat this test with v3.3.2 and still run into issues, we'd be happy to investigate further!

gabors commented 1 year ago

Hi @andrewjl-mux thanks for doing these steps. I can't access your example app, would love to see how you set it up.

I just tried integrating via SPM again using v3.3.2 and was initially getting the same crash on Mac Catalyst.

Just to clarify how my code is structured:

I have a framework target called WBTVDKit (not package) that has the MUXSDKStats package added. This framework has shared code that adds Mux to the video player fwiw. This WBTVDKit framework is linked by an app target called WBTVD-ios. This setup was not loading the Mux SDK, based on the logs.

After I additionally added MUXSDKStats to the app target itself, things started to work! App no longer crashes on start I've replicated the setup for the tvOS app target and it works on real device as well just fine.

Not sure at this point if it was the removing of LC_RPATH that fixed this.

Thanks again!

andrewjl-mux commented 1 year ago

Hi @andrewjl-mux thanks for doing these steps. I can't access your example app, would love to see how you set it up.

Sorry about that! I linked to the PR, which has since been merged and the branch deleted. I've updated the link, let me know you're still not seeing it

andrewjl-mux commented 1 year ago

I just tried integrating via SPM again using v3.3.2 and was initially getting the same crash on Mac Catalyst.

Just to clarify how my code is structured:

I have a framework target called WBTVDKit (not package) that has the MUXSDKStats package added. This framework has shared code that adds Mux to the video player fwiw. This WBTVDKit framework is linked by an app target called WBTVD-ios. This setup was not loading the Mux SDK, based on the logs.

After I additionally added MUXSDKStats to the app target itself, things started to work! App no longer crashes on start I've replicated the setup for the tvOS app target and it works on real device as well just fine.

Not sure at this point if it was the removing of LC_RPATH that fixed this.

Thanks again!

Hmm, very interesting. WBTVDKit is an Xcode framework target, right? Is its built Mach-O type a dynamic library or is it a static library? (Or something else?) You can find that under the MACH_O_TYPE build setting of WBTVDKit target build settings.

I think the issue was that MUXSDKStats was not getting transitively added to the app. You mentioned that WBTVDKit isn't a package, how does it use SPM to install MUXSDKStats?

gabors commented 1 year ago

WBTVDKit is a dynamic library, it's shared between two different streaming apps. This framework target and the app targets are part of the same xcodeproj, that's how they reference all the SPM packages.

andrewjl-mux commented 1 year ago

WBTVDKit is a dynamic library, it's shared between two different streaming apps. This framework target and the app targets are part of the same xcodeproj, that's how they reference all the SPM packages.

Thanks for the additional info! Your workaround manually adding MUXSDKStats to the app target sounds like it should fix this. Crashes due to misconfugured dyld search settings are common to only see on physical iOS / tvOS devices. You mentioned testing physical devices, so the application is able load all required dynamic libraries.

The following is my best guess about the root issue and some alternative options to fix along with caveats. (Your current workaround might be the best one!)

From your explanation, the WBTVDKit dynamic library target is linked to multiple application targets in Xcode without the use of SPM. (Since WBTVDKit isn't itself a Swift package.) WBTVDKit own dependencies were added in Xcode, this time using SPM. Similar to here.

If that's the case ^ then the crash happens because dyld cannot cannot find and load MUXSDKStats at app launch. This is because SPM is changing some build settings, but a few corner cases like Mac Catalyst need manual adjustment. I'm not really sure if this is a bug or an intentional requirement by Xcode.

Because WBTVD-ios build settings weren't (initially, before adding the target) adjusted dyld wasn't able to find and load the MUXSDKStats framework. That explains the dyld errors in the original crash log. When MUXSDKStats was manually added to the WBTVD-ios target , Xcode fixed dyld search paths to include the MUXSDKStats location, and dyld was able to load MUXSDKStats without error.

Possible workarounds

  1. Adding MUXSDKStats to the WBTVD-ios app target, this is what you've done above
  2. Manually configure WBTVD-ios framework search paths and build phases to copy MUXSDKStats when building the app and ensure it is found by dyld (Option 1 does this on your behalf. This option is only if you cannot add the MUXSDKStats target again.)
  3. Convert WBTVDKit to be a Swift package and add it as an SPM dependency to WBTVD-iOS using Xcode. (The example previously mentioned is setup like this.)

Option 1 or 2 will always require some manual adjustment unless a future version of Xcode changes how its linker works. Option 3 delegates the entire dependency chain to SPM.

Removing manual adjustment comes at the cost of one-time project conversion from an Xcode framework to an SPM library that's managed as a Framework target by Xcode. The merits of option 3 depend on how simple vs custom WBTVDKit build settings / build steps are. On the more custom side, option 1 or 2 should be OK with the caveat they'll need manual checks and / or maintenance that gets repeated for each application that uses it.

I'm happy to answer any questions or talk through these options further if you are able to share more about how the project is setup. (And feel free to reach via support@mux.com too.)

Thank you again for reporting this!

gabors commented 1 year ago

Thanks very much again! I'll probably end up doing option 3 at some future point. The shared framework has a lot of old UIKit xibs so I've been putting this off.

andrewjl-mux commented 1 year ago

Something else I came across, if I recall correctly the crash appeared at the same time Xcode introduced a new linker. As mentioned here in the Xcode 15 release notes the pre-Xcode 15 linker can be enabled like so:

The classic linker can still be explicitly requested using -ld64, and will be removed in a future release.

Note: Your workaround to this issue shouldn't require changing back to the pre-Xcode 15 linker. But this is a (temporary, it sounds like) option to try in case the crash reoccurs.

andrewjl-mux commented 1 year ago

Thanks very much again! I'll probably end up doing option 3 at some future point. The shared framework has a lot of old UIKit xibs so I've been putting this off.

Not a problem, glad we were able to figure it out. Good luck with the xibs