[Webkit-unassigned] [Bug 222336] WKScriptMessage can race against WKWebView destruction

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=222336

--- Comment #5 from Chris Dumez <cdumez at apple.com> ---
(In reply to Joshua Wise from comment #4)
> Hi Chris --
> 
> In general, I agree with you: the UIKit API (and the WKWebView API)
> shouldn't be called off the main thread.  I think that `release`ing -- and,
> by extension, `dealloc`ing -- a WKWebView needs to be a special exception to
> this rule, though.  It is very difficult to avoid accidentally causing a
> `[WKWebView dealloc]` off of the main thread: although in this minimal
> reproducer case, a `WKWebView` is directly held off the main thread,
> consider the case in which another object deeply holds a strong reference to
> a `WKWebView`, and another thread happens to hold a strong reference to this
> object.
> 
> It seems like an exceptionally non-obvious pitfall that this can go wrong,
> along three axes of surprise for a programmer:
> 
> 1) There is no textual representation in the source that a disallowed use of
> `WKWebView` will occur: even if a block does not directly make mention of a
> `WKWebView`, the failure can occur.  (Phrased another way, it would be
> expected that it is unsafe to use a `WKWebView` -- but it's not expected
> that it's unsafe to even "think about" a `WKWebView`.)
> 
> 2) Although this is disallowed behavior for other `UIKit` APIs (and objects
> that derive from `UIView`), in practice, deallocation from off the main
> thread does not seem to fail for other `UIKit` APIs.  (This is, admittedly,
> weak justification.  But programmers are, if nothing else, pattern
> recognition machines...)
> 
> 3) There is a violation of an otherwise-reasonable assumption in a
> programmer's mental model that ARC-managed objects should be, in general,
> responsible for avoiding wild pointers (i.e., that an ARC-managed object, if
> it needs to have a longer retain lifecycle than a local scope allows, should
> be managed by the holder of that pointer).
> 
> Although `WKWebView` is technically *allowed* to fail in this fashion by its
> specification, it is probably not a good idea for it to do so
> nondeterministically: it should either be tolerant of this misuse, or
> reliably crash or emit a diagnostic on this misuse [1].
> 
> Technically, it is probably not a bug, if the defect metric is "does
> WKWebView correctly implement the specification".  But if the question is
> "does WKWebView present an interface that avoids footguns", I would probably
> classify this as a defect.
> 
> Thanks for your consideration --
> joshua
> 
> [1] One way that `WKWebView` could emit a diagnostic is particularly
> unpleasant: it could implement `[WKWebView release]` on its own (eventually
> calling into `[NSObject release]`), and call into the particularly-reviled
> `[self retainCount]` to determine whether the system is about to deallocate
> the object.  If it is, and the `release` operation is happening off the main
> thread -- i.e., `[self retainCount] == 1 && ![NSThread isMainThread]` --
> then WKWebView could subsequently assert, or minimally, emit a warning.
> 
> This relies on additional undefined and unpleasant behavior -- `[NSObject
> retainCount]` is specified to be unspecified.  But not only does it *happen*
> to work for this application, examination of the Objective-C runtime reveals
> that it *will* work for this application, too.  Tests of this by swizzling
> `[WKWebView release]` reliably catch this behavior in user applications. 
> (Astute observers may note that this cannot be 100% reliable, and that there
> is another -- smaller -- race condition here.  That's true, but the
> diagnostic now occurs when we 'win' the race, not when we 'lose', and as a
> best-effort method to help users avoid API misuse, that's probably good
> enough.)
> 
> All that said, doing that is not pleasant.  It may just be better to make
> WKWebView tolerant of this misuse :-)

Deallocating a WKWebView (and other WK API objects) on a non-main thread was made safe very recently via Bug 223013.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210317/10be6d47/attachment-0001.htm>