New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SR-6795] Swift 4 KVO swizzling is not thread-safe, causes crashes #3742
Comments
Filed radar://36663633 as well |
cc @Catfish-Man |
I’m told there are updates on the radar. Can you please update this ticket instead? |
Also, I'm quite frankly shocked this bug is still open. It's really quite bad and makes Swift KVO completely unusable in any app where KVO could possibly be occurring on a background thread (which is any app that uses AVFoundation or NSOperation at the very least). |
I was told that the radar had an update where engineering was asking for a full crashlog. I provided that the day after (Back on February 5th) and there haven't been any updates since. |
Comment by Jamie Matthews (JIRA) +1 this is happening in my app as well. Several hundred crashes per week. The scenario for us is the same - the main thread attempts to register a KVO observer at the same time that a background thread calls keyPathsForValuesAffectingValue. Also interesting to note - I'm not using Swift 4, rather Swift 3.2 (Xcode 9.0.1) |
Used to happen to our app as well. We've largely abandoned Swift KVO in favor of the old Obj-C methods :-/ |
I've submitted PR #20103 for this issue. |
Comment by Rob Phillips (JIRA) FYI, Eridius (JIRA User)'s fix still isn't released as of iOS 12.2 / Xcode 10.2.1. We just started using Swift KVO (some on worker threads) and immediately hit thousands of crashes in production; not just in our code but in all code that runs any sort of `Operation`. This means Swift KVO is unusable in any sort of background ops throughout your entire application. Even switching to ObjC (or using `PMKVObserver`) is not enough because other static libs that your app uses might be performing background operations you can't do anything about. All it takes is one observation on a background thread to corrupt all other libs within your app. The only safe work around is to create a `main.swift` file, like suggested in apple/swift#20103 (comment) E.g. 1. Remove the @UIApplicationMain decorator from your AppDelegate 2. Create a main.swift file: import UIKit
import Foundation
private class CrashWorkaround: NSObject {
@objc dynamic var test: String = ""
/// More info: https://github.com/apple/swift/pull/20103
fileprivate static func workaroundSwiftKVOCrash() {
assert(Thread.isMainThread)
let obj = CrashWorkaround()
let observer = obj.observe(\.test, options: [.new]) { (_, _) in
fatalError("Shouldn't be called, value doesn't change")
}
observer.invalidate()
}
}
/// This needs to execute before anything can register KVO in the background
/// so the best way to do this is before the application ever runs
CrashWorkaround.workaroundSwiftKVOCrash()
UIApplicationMain(CommandLine.argc, CommandLine.unsafeArgv, nil, NSStringFromClass(AppDelegate.self)) This will get called immediately after dylib loads but before UIApplicationMain does any work to setup the app delegate, runloop, etc. |
Comment by Dan Stenmark (JIRA) I ran into this crasher today on Swift 5.
|
As best I can tell, this fix didn't make it into Swift 5.0 or 5.1? And since the stdlib and overlays are baked into iOS as of iOS 12.2, I think that means iOS 12.2 ..< 13.0 (maybe higher?) will always suffer from this crash? |
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 apple/swift-corelibs-foundation#3742) Thanks to @jckarter for [suggesting this workaround](https://twitter.com/jckarter/status/1586029206646898688?s=61&t=LJyPWVeXcT9TfIL2zMjFng).
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 apple/swift-corelibs-foundation#3742) Thanks to @jckarter for [suggesting this workaround](https://twitter.com/jckarter/status/1586029206646898688?s=61&t=LJyPWVeXcT9TfIL2zMjFng). [CSDK-503]: https://revenuecats.atlassian.net/browse/CSDK-503?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Environment
Apple Swift version 4.0.3 (swiftlang-900.0.74.1 clang-900.0.39.2)
Target: x86_64-apple-macosx10.9
iOS 10/11
Additional Detail from JIRA
md5: 53a1dd70f800671add744984e248da7d
is duplicated by:
Issue Description:
The method swizzling that Swift 4 KVO does to handle key paths is not thread-safe. Specifically, it's swizzling stuff in the wrong order, such that KVO on another thread at the right time will crash as it ends up invoking the method implementation that looks like
We just released Twitch 5.8 with Swift 4 and we already have several hundred of these crashes, where the main thread is in the process of adding its first Swift 4 KVO observer and AVPlayerLayer on a background thread crashes trying to invoke its own KVO.
The crash backtrace looks like
If you read the Swift 4 KVO swizzling code, it first swizzles
+[NSObject keyPathsForValuesAffectingValueForKey:]
with the new implementation, and as soon as that one method is swizzled, calling it will crash. It's not until a later swizzle that the crash is repaired.The text was updated successfully, but these errors were encountered: