Bug 275368
Summary: | WKWebsiteDataStore thread assertions | ||
---|---|---|---|
Product: | WebKit | Reporter: | Adam Lickel <adam> |
Component: | WebKit API | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED DUPLICATE | ||
Severity: | Normal | CC: | ap, cdumez, ggaren, sihui_liu, wilander |
Priority: | P2 | ||
Version: | Safari 17 | ||
Hardware: | All | ||
OS: | iOS 17 |
Adam Lickel
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
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Adam Lickel
CC wilander (discussed in person at WWDC24)
Alexey Proskuryakov
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 ***
Geoffrey Garen
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.
Geoffrey Garen
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.
Adam Lickel
> 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.
Adam Lickel
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.