This considers both XPCOM's ns(Supports)WeakReference and mfbt/(Supports)WeakPtr. There might be a problem when multiple threads access/reference/dereference an object implementing nsSupportsWeakReference or SupportsWeakPtr. Scenario: - thread A calls Release() and drops refcnt of an object implementing nsSupportsWeakReference to 0, enters the branch that does |delete this| - before thread A reaches the |delete this| line thread B takes and addrefs the referent (pointing to the real object) in the related nsWeakReference object ; this may happen because we set the *referent* to null from the real object's dtor that has not yet been reached at this moment - thread A calls |delete this| on the real object - thread B works with a deleted object (use-after-free) and when it's Release()'ed it may be (depending on in what state the heap is) double deleted (double-free) I raise this problem since I've hit this misunderstanding of weak reference usage by two developers recently: 1: http://hg.mozilla.org/mozilla-central/annotate/325c74addeba/netwerk/base/src/Tickler.h#l56 2: http://hg.mozilla.org/mozilla-central/annotate/325c74addeba/netwerk/cache2/CacheFile.h#l50 Both demonstrates that refcntrs are implemented as thread-safe (and objects are used/can be dereferenced on any thread) but both implement nsSupportsWeakReference/SupportsWeakPtr despite the above described problem. If I'm right with the scenario, we must audit the code and fix all places.
I think you're right! In addition to auditing the existing code, it would be nice if we could do something to fix this and/or protect future callers against it, I just can't think of a way to do either.
You need to log in before you can comment on or make changes to this bug.