[webkit-reviews] review granted: [Bug 174081] [WTF] Implement WTF::ThreadGroup : [Attachment 314961] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 174081: [WTF] Implement WTF::ThreadGroup
https://bugs.webkit.org/show_bug.cgi?id=174081

Attachment 314961: Patch

https://bugs.webkit.org/attachment.cgi?id=314961&action=review




--- Comment #45 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 314961
  --> https://bugs.webkit.org/attachment.cgi?id=314961
Patch

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

r=me with remaining issues addressed.

>> Source/WTF/wtf/Threading.cpp:156
>> +			threadGroups.append(WTFMove(retained));
> 
> How does std::enable_shared_from_this work?  It is ref counted, and is it
thread safe?  Is there a race here with invoking threadGroup.lock() from this
Thread which may be a different thread than then one that the VM (and therefore
ThreadGroup) is running on?  I didn't see anything about thread safety in
http://en.cppreference.com/w/cpp/memory/enable_shared_from_this and
http://en.cppreference.com/w/cpp/memory/weak_ptr.

I looked into the implementation of std::enable_shared_from_this.  It uses
atomics, and is therefore thread-safe.	No issue here.