Bug 275368 - WKWebsiteDataStore thread assertions
Summary: WKWebsiteDataStore thread assertions
Status: RESOLVED DUPLICATE of bug 273798
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: Safari 17
Hardware: All iOS 17
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-06-11 12:54 PDT by Adam Lickel
Modified: 2024-09-10 10:52 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Lickel 2024-06-11 12:54:28 PDT
WKWebsiteDataStore lacks @MainActor annotations. While this is OK for closure-based methods, the auto-generated Swift Concurrency methods will never be called on the Main Thread. This creates a WTF assertion.

I suspect the entire class _should_ be marked as @MainActor to resolve this; however, at a minimum, the Swift Concurrency methods should not be auto-generated.

Presumably, this is fixed on main, per `WK_SWIFT_UI_ACTOR` though the initial Xcode 18 beta is not marked as such.

---

Sample code:

private extension WKWebsiteDataStore {
    private static let canCallDataStoreAPIs = true

    @MainActor
    @available(iOS 17, *)
    static func removeAllPersistentDataStores() async {
//        guard canCallDataStoreAPIs else {
//            return
//        }

        let storageIDs = if canCallDataStoreAPIs {
            await WKWebsiteDataStore.allDataStoreIdentifiers
        } else {
            await WKWebsiteDataStore.safeAllDataStoreIdentifiers
        }

        await withDiscardingTaskGroup { @MainActor group in
            for storageID in storageIDs {
                group.addTask {
                    if canCallDataStoreAPIs {
                        try? await WKWebsiteDataStore.remove(forIdentifier: storageID)
                    } else {
                        try? await WKWebsiteDataStore.safeRemove(forIdentifier: storageID)
                    }
                }
            }
        }
    }

    @MainActor
    @available(iOS 17, *)
    private static var safeAllDataStoreIdentifiers: [UUID] {
        get async {
            await withCheckedContinuation { continuation in
                WKWebsiteDataStore.fetchAllDataStoreIdentifiers { results in
                    continuation.resume(returning: results)
                }
            }
        }
    }

    @MainActor
    @available(iOS 17, *)
    private static func safeRemove(forIdentifier storageID: UUID) async throws {
        try await withCheckedThrowingContinuation { (checkedContinuation: CheckedContinuation<Void, Error>) in
            WKWebsiteDataStore.remove(forIdentifier: storageID) { error in
                if let error {
                    checkedContinuation.resume(throwing: error)
                } else {
                    checkedContinuation.resume()
                }
            }
        }
    }

    @MainActor
    func clearAllWebsiteData() async {
        let records = await dataRecords(
            ofTypes: WKWebsiteDataStore.allWebsiteDataTypes()
        )
        await withTaskGroup(of: Void.self) { group in
            for record in records {
                group.addTask { @MainActor in
                    await self.removeData(ofTypes: record.dataTypes, for: [record])
                }
            }
        }
    }
}

---

allDataStoreIdentifiers stack trace:

Thread 13 Queue : com.apple.WebKit.WebsiteDataStoreIO (serial)
#0	0x00000001953e5204 in WTF::RunLoop::dispatch(WTF::Function<void ()>&&) ()
#1	0x000000018b49ad58 in WTF::Detail::CallableWrapper<WebKit::WebsiteDataStore::fetchAllDataStoreIdentifiers(WTF::CompletionHandler<void (WTF::Vector<WTF::UUID, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&)>&&)::$_8, void>::call() ()
#2	0x0000000195442b8c in void WTF::dispatchWorkItem<WTF::(anonymous namespace)::DispatchWorkItem>(void*) ()
#3	0x000000010228573c in _dispatch_client_callout ()
#4	0x000000010228da30 in _dispatch_lane_serial_drain ()
#5	0x000000010228e774 in _dispatch_lane_invoke ()
#6	0x000000010229b1a8 in _dispatch_root_queue_drain_deferred_wlh ()
#7	0x000000010229a604 in _dispatch_workloop_worker_thread ()
#8	0x0000000103297814 in _pthread_wqthread ()
Enqueued from com.apple.main-thread (Thread 1) Queue : com.apple.main-thread (serial)
#0	0x000000010228a694 in dispatch_async_f ()
#1	0x000000018b495dc4 in WebKit::WebsiteDataStore::fetchAllDataStoreIdentifiers(WTF::CompletionHandler<void (WTF::Vector<WTF::UUID, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&)>&&) ()
#2	0x000000018b3248f4 in +[WKWebsiteDataStore(WKPrivate) _fetchAllIdentifiers:] ()
#3	0x000000010b863994 in static WKWebsiteDataStore.removeAllPersistentDataStores() at XXX

---

remove(forIdentifier:) stack trace:

Thread 8 Queue : com.apple.root.user-initiated-qos.cooperative (concurrent)
#0	0x000000018af15ff8 in WTFCrashWithInfo(int, char const*, char const*, int) ()
#1	0x000000018b69584c in WebKit::allDataStores() ()
#2	0x000000018b696bc4 in WebKit::WebsiteDataStore::existingDataStoreForIdentifier(WTF::UUID const&) ()
#3	0x000000018b496028 in WebKit::WebsiteDataStore::removeDataStoreWithIdentifier(WTF::UUID const&, WTF::CompletionHandler<void (WTF::String const&)>&&) ()
#4	0x000000018b3249f8 in +[WKWebsiteDataStore(WKPrivate) _removeDataStoreWithIdentifier:completionHandler:] ()
#5	0x0000000110294330 in closure #1 in closure #1 in static WKWebsiteDataStore.removeAllPersistentDataStores() at XXX
#6	0x00000001102985f4 in partial apply for closure #1 in closure #1 in static WKWebsiteDataStore.removeAllPersistentDataStores() ()
#7	0x000000011029871c in thunk for @escaping @callee_guaranteed @Sendable @async () -> () ()
#8	0x000000011029885c in partial apply for thunk for @escaping @callee_guaranteed @Sendable @async () -> () ()

---

I have also seen this crash the closure-based method _on_ the main thread, early in the app's lifecycle.
Specifically, this gets called in func application(_:,didFinishLaunchingWithOptions:) in a `Task{}`

Thread 8 Queue : com.apple.WebKit.WebsiteDataStoreIO (serial)
#0	0x00000001953e5204 in WTF::RunLoop::dispatch(WTF::Function<void ()>&&) ()
#1	0x000000018b49ad58 in WTF::Detail::CallableWrapper<WebKit::WebsiteDataStore::fetchAllDataStoreIdentifiers(WTF::CompletionHandler<void (WTF::Vector<WTF::UUID, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&)>&&)::$_8, void>::call() ()
#2	0x0000000195442b8c in void WTF::dispatchWorkItem<WTF::(anonymous namespace)::DispatchWorkItem>(void*) ()
#3	0x000000010348d73c in _dispatch_client_callout ()
#4	0x0000000103495a30 in _dispatch_lane_serial_drain ()
#5	0x0000000103496774 in _dispatch_lane_invoke ()
#6	0x00000001034a31a8 in _dispatch_root_queue_drain_deferred_wlh ()
#7	0x00000001034a2604 in _dispatch_workloop_worker_thread ()
#8	0x00000001033e7814 in _pthread_wqthread ()
Enqueued from com.apple.main-thread (Thread 1) Queue : com.apple.main-thread (serial)
#0	0x0000000103492694 in dispatch_async_f ()
#1	0x000000018b495dc4 in WebKit::WebsiteDataStore::fetchAllDataStoreIdentifiers(WTF::CompletionHandler<void (WTF::Vector<WTF::UUID, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&)>&&) ()
#2	0x000000018b3248f4 in +[WKWebsiteDataStore(WKPrivate) _fetchAllIdentifiers:] ()
#3	0x000000010baf0a48 in closure #1 in static WKWebsiteDataStore.safeAllDataStoreIdentifiers.getter at XXX
#4	0x000000020a881744 in closure #1 in withCheckedContinuation<τ_0_0>(function:_:) ()
#5	0x000000020a88177c in partial apply for closure #1 in withCheckedContinuation<τ_0_0>(function:_:) ()
#6	0x000000020a8815b4 in withUnsafeContinuation<τ_0_0>(_:) ()
#7	0x000000020a8b410c in swift::runJobInEstablishedExecutorContext(swift::Job*) ()
#8	0x000000020a8b4fd4 in swift_job_runImpl(swift::Job*, swift::ExecutorRef) ()
#9	0x000000010349d2a8 in _dispatch_main_queue_drain ()
#10	0x000000010349cf1c in _dispatch_main_queue_callback_4CF ()
#11	0x000000018040e960 in __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ ()
#12	0x0000000180409078 in __CFRunLoopRun ()
#13	0x00000001804084d4 in CFRunLoopRunSpecific ()
#14	0x000000018ef2aae4 in GSEventRunModal ()
#15	0x00000001853d0a28 in -[UIApplication _run] ()
#16	0x00000001853d46b0 in UIApplicationMain ()
#17	0x00000001004f614c in main at XXX/AppDelegate.swift:53
Comment 1 Adam Lickel 2024-06-11 12:56:59 PDT
CC wilander (discussed in person at WWDC24)
Comment 2 Alexey Proskuryakov 2024-06-12 19:47:54 PDT
Yes, I'm told that this was already done in bug 273798, and should be in a seed build soon. Thank you for the report!

*** This bug has been marked as a duplicate of bug 273798 ***
Comment 3 Geoffrey Garen 2024-06-13 09:05:05 PDT
Can you clarify what you meant by "the auto-generated Swift Concurrency methods"?

I believe WebKit's @MainActor annotations (present in main but not in the Seed 1 SDK) address the issue reported here. But I'm trying to figure out if other parts of the SDK may have issues.
Comment 4 Geoffrey Garen 2024-06-13 09:24:17 PDT
Oh, I think I get it now. The "group.addTask" statement in 

> await withDiscardingTaskGroup { @MainActor group in
>     for storageID in storageIDs {
>         group.addTask {
>             if canCallDataStoreAPIs {
>                 try? await WKWebsiteDataStore.remove(forIdentifier: storageID)
>             } else {
>                 try? await WKWebsiteDataStore.safeRemove(forIdentifier: storageID)
>             }
>         }
>     }
> }

means "go do this on a secondary thread". That's the whole bug. WKWebsiteDataStore.remove must be called on the main thread. Once the @MainThread annotation is in the SDK, this code will (correctly) fail to compile.

Side point: There's no need for any of this surrounding async-ifying code. WKWebsiteDataStore will do the expensive work async, and call you back when it's done. Additional async-ifying around that is just overhead.
Comment 5 Adam Lickel 2024-06-13 10:41:42 PDT
> Can you clarify what you meant by "the auto-generated Swift Concurrency methods"?
>
> I believe WebKit's @MainActor annotations (present in main but not in the Seed 1 SDK) address the issue reported here. But I'm trying to figure out if other parts of the SDK may have issues.

I agree, except maybe the assertion that occurs in AppDelegate.appDidFinishLaunching() which was still crashing when the call was made on the Main Thread.

> There's no need for any of this surrounding async-ifying code. 

The intention was to delete all of these things in “parallel” rather than iteratively dispatching requests. 

My assumption is it works roughly as:
* get file on main thread
* off main thread delete the File object
* return status on main thread

If I perform this in parallel then the N File deletions could occur simultaneously.

If the underlying File IO will be performed iteratively in a single threaded queue, then I will remove the Task Group.
Comment 6 Adam Lickel 2024-09-10 10:52:28 PDT
I'm still experiencing this with WKWebsiteDataStore.fetchAllDataStoreIdentifiers() / WKWebsiteDataStore.allDataStoreIdentifiers.

I initially tried it as part of AppDelegate.application(_:,didFinishLaunchingWithOptions:)

I then added an explicit `DispatchQueue.main.async {}` + `withCheckedContinuation` to call the closure-based method on the next run loop.

Both crash in the WTF handler with the Xcode 16 Release Candidate / iOS 18.