fix(macos): prevent use-after-free in stop_task on macOS 11 during WKWebView dealloc#1734
fix(macos): prevent use-after-free in stop_task on macOS 11 during WKWebView dealloc#1734x93008 wants to merge 2 commits into
Conversation
macOS 11 WebKit bug: during WKWebView dealloc, stopAllTasksForPage calls stop_task with already-freed task pointers. Any access (including the implicit objc_release from objc2 reference types) causes SIGSEGV. Fix: - stop_task: use raw pointers (*mut AnyObject) instead of objc2 references to skip automatic retain/release. Body is no-op since task is invalid. - start_task response handler: explicit drop(webview) before drop(task) to ensure correct deallocation order.
2c26a8b to
e8cf34d
Compare
Tested Platforms
All platforms pass without crash. |
e8cf34d to
9be1a6b
Compare
|
Without knowing a lot about the platform, it would be very odd for pointers passed from the platform to user code to be invalid. The manual drop order looks really wrong as well, a mixture between manual memory management and refcounting. |
Package Changes Through 9be1a6bThere are 1 changes which include wry with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
|
Thanks for the feedback. Let me address both points: Re: "it would be very odd for pointers passed from the platform to user code to be invalid" You're right that this shouldn't happen, but it's a known WebKit bug that was fixed upstream. The specific issue is in
The fix added See also #1484 which fixed a closely related issue on the Re: "The manual drop order looks really wrong" I agree. The explicit drop ordering is a hack and I'm not confident it's the right approach. To be honest, I'm not deeply familiar with objc2's reference semantics in Rust or the idiomatic way to handle this kind of platform-level lifetime issue. My fix was pragmatic (it stops the crash across macOS 11–26) but clearly not elegant. If someone more experienced with Rust + macOS/objc2 could suggest a cleaner solution, I'd be happy to rework the PR accordingly. The core constraint is: on macOS 11, |
Legend-Master
left a comment
There was a problem hiding this comment.
Also thanks for the context,
FIXME: though we give it a static lifetime, it's not guaranteed to be valid.
is a leftover from #1484 and can be removed now
| } | ||
| }; | ||
|
|
||
| // webview must drop before task: if webview drop triggers dealloc → |
There was a problem hiding this comment.
Does this alone fix the problem?
There was a problem hiding this comment.
Is this change still necessary once we figure out what to do with stop_task?
I'm reading the explanation as, dropping webview is necessary because we call stop_task on the freed task, which the other hunk is supposed to address.
| _task: *mut AnyObject, | ||
| ) { | ||
| webview.remove_custom_task_key(task.hash()); | ||
| // no-op: avoid accessing task/webview — macOS 11 may pass freed pointers here |
There was a problem hiding this comment.
I don't think we can simply removing this?
macOS 11 WebKit bug: during WKWebView dealloc,
stopAllTasksForPagecallsstop_taskwith already-freed task pointers. Any access (including the implicitobjc_releasefrom objc2 reference types) causes SIGSEGV.Fix:
stop_task: use raw pointers (*mut AnyObject) instead of objc2 references to skip automatic retain/release. Body is no-op since task is invalid.start_taskresponse handler: explicitdrop(webview)beforedrop(task)to ensure correct deallocation order.closes #1733
Details
On macOS 11, WebKit's internal
stopAllTasksForPage(triggered during[WKWebView dealloc]) passes already-freedWKURLSchemeTaskpointers to thewebView:stopURLSchemeTask:callback. Using objc2 reference types (&WryWebView,&ProtocolObject<dyn WKURLSchemeTask>) triggers implicitobjc_retainon entry, which crashes on the freed pointer.The explicit drop ordering (
drop(webview)beforedrop(task)) prevents a scenario where droppingRetained<WryWebView>on a worker thread triggers dealloc →stopAllTasksForPage→stop_task, while the task reference is still alive in the closure.This is the same class of race condition reported in tauri-apps/tauri#11516 (macOS 15) and #1730.