Skip to content

NetworkOperation: workaround for iOS 12 crashes#2008

Merged
NachoSoto merged 1 commit into
mainfrom
network-operation-ios-12-workaround
Oct 28, 2022
Merged

NetworkOperation: workaround for iOS 12 crashes#2008
NachoSoto merged 1 commit into
mainfrom
network-operation-ios-12-workaround

Conversation

@NachoSoto

@NachoSoto NachoSoto commented Oct 28, 2022

Copy link
Copy Markdown
Contributor

This might fix CSDK-503.

The latest stacktrace provided gave me a clue:

Fatal error: Should never be reached: file /BuildRoot/Library/Caches/com.apple.xbs/Sources/swiftlang_overlay_Foundation_Device/swiftlang-1001.2.63.13/swift/stdlib/public/SDK/Foundation/NSObject.swift, line 46
0xa3d10 @objc static NSObject.__old_unswizzled_keyPathsForValuesAffectingValue(forKey:) + 84
53  libswiftFoundation.dylib       0xa436c static __KVOKeyPathBridgeMachinery.keyPathsForValuesAffectingValue(forKey:) + 272
64  libswiftFoundation.dylib       0xa4864 @objc static __KVOKeyPathBridgeMachinery.keyPathsForValuesAffectingValue(forKey:) + 88

That crash is coming from here.
KVO shipped broken on Swift at least throughout iOS 12.x (see swiftlang/swift-corelibs-foundation#3742)

Thanks to @jckarter for suggesting this workaround.

@NachoSoto NachoSoto added the pr:fix A bug fix label Oct 28, 2022
@NachoSoto NachoSoto requested a review from a team October 28, 2022 16:39

@aboedo aboedo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG 🤦🏻‍♂️

I'm so glad we have a workaround!! 🎉🎉🎉

I'd just add some explanation for future us and 🚢

Comment on lines 45 to 51

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's definitely leave a note here explaining why it's important to use keyPath here, there's no way a maintainer would guess it if they're not familiar

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can even link to the discussion with a rebrandly link or something

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@NachoSoto

Copy link
Copy Markdown
Contributor Author

I'll update with a comment.

I forgot to mention that this is obviously well tested. Removing the KVO lines breaks all sorts of tests so there's good coverage of this.

This _might_ fix [CSDK-503].
The latest stacktrace provided gave me a clue:
> Fatal error: Should never be reached: file /BuildRoot/Library/Caches/com.apple.xbs/Sources/swiftlang_overlay_Foundation_Device/swiftlang-1001.2.63.13/swift/stdlib/public/SDK/Foundation/NSObject.swift, line 46
> 0xa3d10 @objc static NSObject.__old_unswizzled_keyPathsForValuesAffectingValue(forKey:) + 84
>53  libswiftFoundation.dylib       0xa436c static __KVOKeyPathBridgeMachinery.keyPathsForValuesAffectingValue(forKey:) + 272
>64  libswiftFoundation.dylib       0xa4864 @objc static __KVOKeyPathBridgeMachinery.keyPathsForValuesAffectingValue(forKey:) + 88

That crash is coming from [here](https://github.com/apple/swift-corelibs-foundation/blob/swift-DEVELOPMENT-SNAPSHOT-2022-10-24-a/Darwin/Foundation-swiftoverlay/NSObject.swift#L47).

KVO shipped broken on Swift at least throughout iOS 12.x (see swiftlang/swift-corelibs-foundation#3742)

Thanks to @jckarter for [suggesting this workaround](https://twitter.com/jckarter/status/1586029206646898688?s=61&t=LJyPWVeXcT9TfIL2zMjFng).
@NachoSoto NachoSoto requested a review from aboedo October 28, 2022 18:52
@NachoSoto NachoSoto force-pushed the network-operation-ios-12-workaround branch from 324f9d3 to 398158e Compare October 28, 2022 18:52
@NachoSoto NachoSoto merged commit f26bc64 into main Oct 28, 2022
@NachoSoto NachoSoto deleted the network-operation-ios-12-workaround branch October 28, 2022 18:59
@NachoSoto

Copy link
Copy Markdown
Contributor Author

Thanks Elon.

@RCGitBot RCGitBot mentioned this pull request Nov 3, 2022
NachoSoto pushed a commit that referenced this pull request Nov 4, 2022
### New Features
* Introduced `PurchasesDiagnostics` to help diagnose SDK configuration
errors (#1977) via NachoSoto (@NachoSoto)
### Bugfixes
* Avoid posting empty receipts by making`TransactionsManager` always use
`SK1` implementation (#2015) via NachoSoto (@NachoSoto)
* `NetworkOperation`: workaround for iOS 12 crashes (#2008) via
NachoSoto (@NachoSoto)
### Other Changes
* Makes hold job wait for installation tests to pass (#2017) via Cesar
de la Vega (@vegaro)
* Update fastlane-plugin-revenuecat_internal (#2016) via Cesar de la
Vega (@vegaro)
* `bug_report.md`: changed SK2 wording (#2010) via NachoSoto
(@NachoSoto)
* Added `Set + Set` and `Set += Set` operators (#2013) via NachoSoto
(@NachoSoto)
* fix the link to StoreKit Config file from watchOS purchaseTester
(#2009) via Andy Boedo (@aboedo)
* `PurchaseTesterSwiftUI`: combined targets into one multi-platform and
fixed `macOS` (#1996) via NachoSoto (@NachoSoto)
* Less Array() (#2005) via SabinaHuseinova (@SabinaHuseinova)
* Docs: fixed `logIn` references (#2002) via NachoSoto (@NachoSoto)
* CI: use `Xcode 14.1` (#1992) via NachoSoto (@NachoSoto)
* `PurchaseTesterSwiftUI`: fixed warnings and simplified code using
`async` methods (#1985) via NachoSoto (@NachoSoto)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants