Bug 303024 – REGRESSION (300778@main, iOS 26.1): Crashes in STScreenTimeConfigurationObserver

• Component: WebKit Misc.
• Status: RESOLVED FIXED

It seems commit https://github.com/WebKit/WebKit/commit/00f048145285ac13fc9a26dcd37556fb5dc5e44a add kvo of STScreenTimeConfigurationObserver.configuration.enforcesChildRestrictions bring in  a random crash below

Diagnosis:Application threw exception NSInternalInconsistencyException: Cannot update for observer <WKWebView 0x12ced7000> for the key path "configuration.enforcesChildRestrictions" from <STScreenTimeConfigurationObserver 0x12f2f6700>, most likely because the value for the key "configuration" has changed without an appropriate KVO notification being sent. Check the KVO-compliance of the STScreenTimeConfigurationObserver class.



stacktrace

___exceptionPreprocess
_objc_exception_throw
-[NSKeyValueNestedProperty object:withObservance:didChangeValueForKeyOrKeys:recurse:forwardingValues:]
_NSKeyValueDidChange
-[NSObject(NSKeyValueObservingPrivate) _changeValueForKeys:count:maybeOldValuesDict:maybeNewValuesDict:usingBlock:]
-[NSObject(NSKeyValueObservingPrivate) _changeValueForKey:key:key:usingBlock:]
__NSSetObjectValueAndNotify
___58-[STScreenTimeConfigurationObserver _requestConfiguration]_block_invoke_2
___invoking___
-[NSInvocation invoke]
___NSXPCCONNECTION_IS_CALLING_OUT_TO_EXPORTED_OBJECT__
-[NSXPCConnection _decodeAndInvokeReplyBlockWithEvent:sequence:replyInfo:]
___88-[NSXPCConnection _sendInvocation:orArguments:count:methodSignature:selector:withProxy:]_block_invoke_5
__xpc_connection_reply_callout
__xpc_connection_call_reply_async
__dispatch_client_callout3_a
__dispatch_mach_msg_async_reply_invoke
__dispatch_root_queue_drain_deferred_item
__dispatch_kevent_worker_thread
__pthread_wqthread
_start_wqthread

[SUNDX]

same crash here

[Alexey Proskuryakov]

Thank you for the report! Could you please add some information about the circumstances where you see it? E.g. is it in Safari, or in other clients? Should it be possible to attach a full .ips crash report?

[Radar WebKit Bug Importer]

<rdar://problem/165373221>

(In reply to Alexey Proskuryakov from comment #2)
> Thank you for the report! Could you please add some information about the
> circumstances where you see it? E.g. is it in Safari, or in other clients?
> Should it be possible to attach a full .ips crash report?

It's in out own client,and there's no ips crash file right now cause we can't reproduce the crash locally.

(In reply to Alexey Proskuryakov from comment #2)
> Thank you for the report! Could you please add some information about the
> circumstances where you see it? E.g. is it in Safari, or in other clients?
> Should it be possible to attach a full .ips crash report?

May I ask if there is any progress at present? The number of crash cases on our side is gradually increasing.

What is happening:

This crash is happening because the update queue utilized for STScreenTimeConfigurationObserver is concurrent instead of serial.
See the highlighted line introduced in https://github.com/WebKit/WebKit/commit/00f048145285ac13fc9a26dcd37556fb5dc5e44a#diff-bffab4ea0adef52dddc198a8754e422d28c9154ab42b312527fa79accd1abe75R428-R429.

What is the fix:
Long story short is to utilize the main queue or a private serial one.

Why is this happening:
The Automatic KVO implementation for STScreenTimeConfigurationObserver is not thread safe. While STScreenTimeConfiguration is.
See: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/KeyValueObserving/Articles/KVOImplementation.html#//apple_ref/doc/uid/20002307-BAJEAIEE on what automatic KVO is.

This issue can be reproduced by concurrently setting the private method [STScreenTimeConfigurationObserver setConfiguration: configuration] to a new configuration.
In the implementation of STScreenTimeConfigurationObserver, the configuration is nil on setup. And the first callback from the underlying NSEXCConnection calls [STScreenTimeConfigurationObserver setConfiguration:]. Further updates call the private method [STScreenTimeConfigurationObserver _updateWithConfiguration:] which update the instance of STScreenTimeConfiguration directly where its kvo updates are thread safe. Basically [STScreenTimeConfigurationObserver setConfiguration:] is getting called multiple times at once.


How to reproduce:
```
 //AppDelegate.swift
let observer = MyObserver()
observer.startObservation
```


```
// MyObserver.swift
@preconcurrency import ScreenTime
import WebKit

private nonisolated(unsafe) var screenTimeConfigurationObserverKVOContext: UInt8 = 0

/// Observer instance to mimic the WKScreenTimeConfigurationObserver in WebKit.
@MainActor
final class MyObserver: NSObject {

    private var observer: STScreenTimeConfigurationObserver?

    // The same queue that it utilized by WebKit
   // Change to main or serial to fix
    private let queue = DispatchQueue.global(qos: .default)

    func startObserving() {

        guard observer == nil else {
            return
        }

        let observer = STScreenTimeConfigurationObserver(
            updateQueue: queue // DANGER: A concurrent queue is passed in instead of serial.
        )
        self.observer = observer

        // The class is STScreenTimeConfigurationObserver
        print("Observer BEFORE KVO: \(String(cString: object_getClassName(observer)))")

        observer.addObserver(
            self,
            forKeyPath: "configuration.enforcesChildRestrictions",
            options: [],
            context: &screenTimeConfigurationObserverKVOContext
        )

        // The class is NSKVONotifying_STScreenTimeConfigurationObserver
        print("Observer After KVO: \(String(cString: object_getClassName(observer)))")

        // That means there is an "sudo" override of setConfiguration.
        // The actual code in the stack trace is _NSSetObjectValueAndNotify.
        // This is logically the same.
        // - (void) setConfiguration: STScreenTimeConfiguration* {
        //    [self willChangeValueForKey: @"configuration"] // not thread safe
        //    [super setConfiguration: configuration] // thread safe
        //    [self didChangeValueForKey: @"configuration"] // not thread safe
        // }


        // If the configuration is set to a non nil property before hand, crash doesn't occurs.
        // call me before observer.startObserving() is called.
        // This works b/c STScreenTimeConfigurationObserver does not create a dynamic KVO subclass.
        // It's properties are "safe" from crash but there are still bad protections.

        // UNCOMMENT ME TO FIX:
        observer.set(configuration: .create(enforcesChildRestrictions: false))

        observer.startObserving() // call not need to reproduce.

        simulateMultipleXPCConnectionUpdates()
    }

    // Simulate multiple calls from the underlying XPC Service at the same time b/c a the concurrent queue
    private func simulateMultipleXPCConnectionUpdates() {
        guard let observer else { return }
        for _ in 0..<1_000 {
            queue.asyncAfter(deadline: .now()) {
                let enforcesChildRestrictions = arc4random() % 2 == 0
                observer.set(
                    configuration: .create(
                        enforcesChildRestrictions: enforcesChildRestrictions
                    )
                )
                // The method observer.update(configuration:) doesn't have the crash
            }
        }
    }

    func stopObserving() {
        observer?.stopObserving()
        observer?.removeObserver(
            self,
            forKeyPath: "configuration.enforcesChildRestrictions",
            context: &screenTimeConfigurationObserverKVOContext
        )
        observer = nil
    }

    nonisolated override func observeValue(
        forKeyPath keyPath: String?,
        of object: Any?,
        change: [NSKeyValueChangeKey : Any]?,
        context: UnsafeMutableRawPointer?
    ) {
        if context == &screenTimeConfigurationObserverKVOContext {
            let enforcesChildRestrictions = (object as? STScreenTimeConfigurationObserver)?.configuration?.enforcesChildRestrictions
            let valueString = enforcesChildRestrictions?.description ?? "nil"
            print("observeValue(forKeyPath: \(keyPath ?? "nil"), value: \(valueString))")
        } else {
            super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context)
        }
    }
}

extension STScreenTimeConfigurationObserver {

    /// Calls private `[STScreenTimeConfigurationObserver: _updateWithConfiguration]` method
    func update(configuration: STScreenTimeConfiguration) {
        let selector = NSSelectorFromString("_updateWithConfiguration:")
        guard self.responds(to: selector) else {
            fatalError("instances doesn't responds to selector: \(selector)")
        }
        self.perform(selector, with: configuration)
    }

    /// Calls private `[STScreenTimeConfigurationObserver: setConfiguration:]` method
    func set(configuration: STScreenTimeConfiguration) {
        let selector = NSSelectorFromString("setConfiguration:")
        guard self.responds(to: selector) else {
            fatalError("instances doesn't responds to selector: \(selector)")
        }
        self.perform(selector, with: configuration)
    }
}

extension STScreenTimeConfiguration {
    /// Creates an STScreenTimeConfiguration instance using the private initializer
    /// - Parameter enforcesChildRestrictions: Whether to enforce child restrictions
    /// - Returns: A configured STScreenTimeConfiguration instance
    static func create(enforcesChildRestrictions: Bool) -> STScreenTimeConfiguration {
        let allocSelector = NSSelectorFromString("alloc")

        // Allocate instance using perform
        guard let uninitializedConfig = STScreenTimeConfiguration.perform(allocSelector)?.takeUnretainedValue() else {
            fatalError("Failed to allocate STScreenTimeConfiguration instance")
        }

        let selector = NSSelectorFromString("initWithEnforcesChildRestrictions:")

        // Check if selector exists
        guard uninitializedConfig.responds(to: selector) else {
            fatalError("Private initializer 'initWithEnforcesChildRestrictions:' not found on STScreenTimeConfiguration")
        }

        // Get the method implementation
        guard let method = class_getInstanceMethod(object_getClass(uninitializedConfig), selector) else {
            fatalError("Failed to get method implementation for 'initWithEnforcesChildRestrictions:'")
        }

        let implementation = method_getImplementation(method)

        // Cast to the appropriate function signature
        // Signature: (id self, SEL _cmd, BOOL enforcesChildRestrictions) -> id
        typealias InitFunction = @convention(c) (AnyObject, Selector, Bool) -> AnyObject

        let initFunction = unsafeBitCast(implementation, to: InitFunction.self)

        // Call the function
        let initialized = initFunction(uninitializedConfig, selector, enforcesChildRestrictions)

        guard let config = initialized as? STScreenTimeConfiguration else {
            fatalError("Initialization returned unexpected type")
        }

        return config
    }

    // Calls the private `[STScreenTimeConfiguration setEnforcesChildRestrictions]`
    func set(enforcesChildRestrictions: Bool) {
        let selector = NSSelectorFromString("setEnforcesChildRestrictions:")
        guard self.responds(to: selector) else {
            fatalError("instances doesn't responds to selector: \(selector)")
        }

        self.perform(selector, with: enforcesChildRestrictions)
    }
}
```

Filed under FB21862078

Pull request: https://github.com/WebKit/WebKit/pull/57864

[EWS]

Committed 307131@main (70e39a01eceb): <https://commits.webkit.org/307131@main>

Reviewed commits have been landed. Closing PR #57864 and removing active labels.

[EWS]

merge-queue failed to commit PR to repository. To retry, remove any blocking labels and re-apply merge-queue label