[Webkit-unassigned] [Bug 238871] Multiple uses of ThreadSafeRefCounted and CanMakeWeakPtr with non-thread-safe WeakPtr

[bugzilla-daemon at webkit.org]

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

--- Comment #7 from Chris Dumez <cdumez at apple.com> ---
(In reply to Kimmo Kinnunen from comment #4)
> (In reply to Chris Dumez from comment #2)
> > Not that in general, it is perfectly OK for a class to subclass
> > ThreadSafeRefCounted and CanMakeWeakPtr and it is not indicative of a bug.
> > It really depends on how you're using WeakPtr.
> > 
> > E.g., the following case is pretty common in WebKit:
> > 1. We have an object A that gets constructed and destroyed on the main
> > thread.
> > 2. We sometimes pass object A to a background thread T and then back to the
> > main thread.
> > 
> > It is OK as long as you only check the WeakPtr on the main thread and make
> > sure that the object is always destroyed on the main thread.
> 
> No, incorrect.
> If you can guarantee that first ref is taken in main thread and last ref is
> dropped in main thread, then it should not be ThreadSafeRefCounted.

2 notes here:
1. You don't have to drop the last ref on the main thread to guarantee that the object gets destroyed on the main thread. ThreadSafeRefCounted has a template parameter to guarantee that the object gets destroyed on the main thread no matter on what thread the last ref gets dropped.
2. It isn't uncommon for people to dispatch a Ref<> to a ThreadSafeRefCounted object to a background thread and then dispatch it back to the main thread.

> 
> There's no case ThreadSafeRefCounted and CanMakeWeakPtr are correct.

I am not yet convinced, see comment above.

And again, we have threading assertions in place in WeakPtr to make sure the WeakPtr is only used on the thread the WeakPtrFactory was created on. That's not to say that we don't have bugs in our code base but I don't think it is correct to say that all usage of WeakPtr + ThreadSafeRefCounted is wrong. Maybe it would help to talk about a specific example in our code base of where you believe there is a bug? May be easier than talking in the abstract. Again, if we look I believe we can find them and fix those specific cases.

-- 
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/20220406/fa4bb880/attachment-0001.htm>