CheckedObserver is not always safe |
||
Issue descriptionCheckedObserver tries to use WeakPtr to ensure the observer won't be called after it is destroyed. Best practice is to ensure the WeakPtrFactory is the first thing destroyed by making it the last member of a class. Since CheckedObserver is a base class, the derived class may be destructing before the weak ptr is null.
,
Oct 5
Is there a use-case currently where this could be a problem? The design doc linked from Issue 842987 highlighted this in the 'Meta' section """ The WeakPtrFactory is also held in a parent class, and there may be multiple of these. So, e.g, weak pointers may be invalidated after the concrete type is already partially destroyed. But base::ObserverList is the only consumer of these WeakPtrFactory (encapsulation enforces this). This could still have impact: there is precedent for observers being notified once observer destruction has begun. Cases where a pure-virtual call may occur will still be detected (i.e. crash) - it’s not likely that the WeakPtr will add complexity. """ Unless there's a real situation that comes up in Chrome, I think this WontFix. Or "CantFix" :). (unless you have a suggestion?) - feel free to reopen. E.g. Note that cases where destruction of a CheckedObserver triggers an ObserverList notify on itself are unlikely to be hiding the kinds of UAF bugs we are interested in, since it's probable that the ObserverList itself is also being destroyed. That is, the patterns were consumers "leak" an observer tend not to have an ownership model where the thing being Observed is actually owned by the Observer. Of course, that's not guaranteed, and notifying for things other than destruction of the Observee, during destruction of the Observee, are also possible.
,
Oct 5
* Of course, that's not guaranteed, and notifying for things other than destruction of the Observee, during destruction of the Observer, are also possible |
||
►
Sign in to add a comment |
||
Comment 1 by jamescook@chromium.org
, Oct 4