[Webkit-unassigned] [Bug 199857] [WTF] Thread::removeFromThreadGroup leaks weak pointers.

[bugzilla-daemon at webkit.org]

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

Yusuke Suzuki <ysuzuki at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #374293|review?                     |review-
              Flags|                            |

--- Comment #4 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 374293
  --> https://bugs.webkit.org/attachment.cgi?id=374293
Patch

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

Nice catch! Discussed with Mark about whether this works with the current protocol of ThreadGroup and Thread, and we think that the key is that we should remove ThreadGroup unconditionally without using `weakPtr.lock()`. To achieve it simply with O(1), I commented about the HashMap based approach here. Does it seem good for you too?

> Source/WTF/wtf/Threading.cpp:259
>              return sharedPtr.get() == &threadGroup;
>          return false;
>      });

Nice catch! I think the key is that, if we can just remove threadGroup unconditionally from this list, it should work.
So, ideally, the code should be like,

return &threadGroup == weakPtr.get();

Then it works. But the problem is that std::weak_ptr does not have an original raw pointer. So, can you change the code like,

1. Instead of holding a Vector<std::weak_ptr<ThreadGroup>>, let's hold HashMap<ThreadGroup*, std::weak_ptr<ThreadGroup>>
2. Remove the entry from that hash map in this function
3. In L214, iterate the hash map and collect shared_ptr from the value's weak_ptr

Or, another simple way is that holding struct { ThreadGroup* threadGroup; std::weak_ptr<ThreadGroup> weakPtr } in a Vector, but I think HashMap one is better.

-- 
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/20190727/f390bac1/attachment.html>