Bug 107105

Summary: Teach Functional.h about WeakPtr
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, benjamin, eric, ojan.autocc, tonyg, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 106127    
Attachments:
Description Flags
Patch none

Description Adam Barth 2013-01-17 01:25:31 PST 
Teach Functional.h about WeakPtr
Comment 1 Adam Barth 2013-01-17 01:28:24 PST 
Created attachment 183144 [details]
Patch
Comment 2 Eric Seidel (no email) 2013-01-17 01:28:42 PST 
Anders will want to see this. :)
Comment 3 Eric Seidel (no email) 2013-01-17 01:31:14 PST 
Comment on attachment 183144 [details]
Patch

This seems super-useful.
Comment 4 Darin Adler 2013-01-17 09:29:48 PST 
Comment on attachment 183144 [details]
Patch

Anders, are you going to review?
Comment 5 Anders Carlsson 2013-01-17 10:11:30 PST 
Comment on attachment 183144 [details]
Patch

Nice, I didn't even know we had a WeakPtr type. I'd like to know about its semantics:

Is it thread-safe? If not, it seems like this is a bad idea since we're essentially passing objects between two threads here. If it is, then it seems like there needs to be an atomic operation to temporarily make the weak pointer strong so we're not trying to call it on one thread while it's being destroyed on another thread.
Comment 6 Adam Barth 2013-01-17 10:22:46 PST 
> Nice, I didn't even know we had a WeakPtr type.

I just added it a couple days ago:

http://trac.webkit.org/browser/trunk/Source/WTF/wtf/WeakPtr.h

> I'd like to know about its semantics:
> 
> Is it thread-safe? If not, it seems like this is a bad idea since we're essentially passing objects between two threads here. If it is, then it seems like there needs to be an atomic operation to temporarily make the weak pointer strong so we're not trying to call it on one thread while it's being destroyed on another thread.

There are two pieces to a WeakPtr: (1) the WeakPtr itself and (2) the underlying WeakReference.  The WeakReference is ThreadSafeRefCounted, which means you can hold a WeakPtr to an object on any thread.  However, access to the referenced object itself is not thread-safe because there is a race between the "clear" function, which zeros out the raw pointer to the object, and the "get" function, which retrieves the raw pointer.  That means, "get" and "clear" must always be called on the same thread.  WeakReference has ASSERTs to enforce this invariant.

From the point of view of Functional, that means that you can call "bind" on any thread (assuming you have a WeakPtr to the object), but you can only actually run the Function on the object's "home" thread (the thread on which the object will be destroyed---the same one whose identifier is stored in WeakReference::m_boundThread).  If you try to run the Function on the wrong thread, you'll hit an ASSERT in WeakPtr.h.
Comment 7 Anders Carlsson 2013-01-17 13:47:36 PST 
Comment on attachment 183144 [details]
Patch

Thanks, that makes sense.
Comment 8 Adam Barth 2013-01-17 13:48:46 PST 
Comment on attachment 183144 [details]
Patch

Thanks for the review.  :)
Comment 9 WebKit Review Bot 2013-01-17 14:18:38 PST 
Comment on attachment 183144 [details]
Patch

Clearing flags on attachment: 183144

Committed r140040: <http://trac.webkit.org/changeset/140040>
Comment 10 WebKit Review Bot 2013-01-17 14:18:42 PST 
All reviewed patches have been landed.  Closing bug.