Bug 223013 – [Cocoa] Make WebKit API Objective-C objects safe to release on non-main threads

• Component: WebKit API
• Status: RESOLVED FIXED

[Darin Adler]

[Cocoa] Make WebKit API Objective-C objects safe to release on non-main threads

[Darin Adler]

Created attachment 422840 [details]
Patch

[Geoffrey Garen]

Comment on attachment 422840 [details]
Patch

r=me

[Chris Dumez]

Comment on attachment 422840 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422840&action=review

> Source/WTF/wtf/MainThread.cpp:71
> +void ensureOnMainRunLoop(Function<void()>&& function)

Nice name, better than what I had suggested :)

[Darin Adler]

Committed r274227 (235138@main): <https://commits.webkit.org/235138@main>

[Radar WebKit Bug Importer]

<rdar://problem/75275804>

[Joshua Wise]

Hi Darin --

As someone who just tripped on a related issue recently, thanks much for thinking about this!  Unfortunately, I don't think this makes releasing as safe as it appears to.  Bug 222336 has a reproducer that I think will crash even with this fix -- and comment 6 in that bug describes why that crash occurs.

Thanks --
joshua

Have you considered using something like _OBJC_SUPPORTED_INLINE_REFCNT_WITH_DEALLOC2MAIN (see <https://opensource.apple.com/source/objc4/objc4-493.11/runtime/objc-internal.h.auto.html>) here? I believe that would account for cases like bug 222336.

[Darin Adler]

I did not know about _OBJC_SUPPORTED_INLINE_REFCNT_WITH_DEALLOC2MAIN.

[Joshua Wise]

I believe that `_OBJC_SUPPORTED_INLINE_REFCNT_LOGIC` is vulnerable to the same weak pointer race condition as bug 222336 (though I have not tried it).  The problem is that `_isDeallocating` causes a weak pointer to fail to store, but `dealloc` causes a weak pointer to be zeroed.  Or, to put it another way, `dealloc2main` is not sufficient: `release2main` is necessary.

But I, also, did not know about `_OBJC_SUPPORTED_INLINE_REFCNT_LOGIC`.  I confess that I find it quite disconcerting that someone would believe this to be common enough of an idiom to make a macro for it, and even more disconcerting that that an invocation variant exists with `release2main == 0`.  It is perhaps fitting that one of the only textual references to this on the web is from seven years ago on, of all places, the SomethingAwful forums.