Bug 195656 - Add a thread safety assertion for RefCounted
Summary: Add a thread safety assertion for RefCounted
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-12 17:39 PDT by Ryosuke Niwa
Modified: 2019-03-13 10:41 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 Ryosuke Niwa 2019-03-12 17:39:25 PDT
I think we should add some thread safety check for RefCounted.
Namely, we should avoid creating an object and ref / deref it in two different threads.
Such objects should be using ThreadSafeRefCounted instead.
Comment 1 Ryosuke Niwa 2019-03-12 17:40:33 PDT
Note that it's technically correct to create an object in one thread, pass it to another thread if the new thread has an exclusive ownership / access right. However, such a code is error prone so I'd argue that we shouldn't have such a code without such code using ThreadSafeRefCounted instead.
Comment 2 Fujii Hironori 2019-03-12 19:28:17 PDT
WTF::String has a non-atomic ref couter, but it is a quite common pattern to pass a WTF::String cross-threads by using isolatedCopy().
Comment 3 Ryosuke Niwa 2019-03-12 19:31:37 PDT
(In reply to Fujii Hironori from comment #2)
> WTF::String has a non-atomic ref couter, but it is a quite common pattern to
> pass a WTF::String cross-threads by using isolatedCopy().

That's a good point. Maybe we need to allow this usage in some classes, or perhaps isolatedCopy needs some magic to unset the thread identity.
Comment 4 Geoffrey Garen 2019-03-13 10:41:05 PDT
We used to have assertions like this and we had to remove them. The corner case was JavaScriptCore. JavaScriptCore allows clients to run on any thread, and it ensures thread safety through coarse-grained locking. JavaScriptCore also uses many fundamental WTF types. We could not find a practical way to annotate all types as asserting their thread affinity if and only if not used by JavaScriptCore.

But perhaps the opposite is possible: WebCore could annotate certain important objects upon allocation as being tied to a thread.