[Webkit-unassigned] [Bug 145978] Media Session: Active participating elements can change while being iterated

[bugzilla-daemon at webkit.org]

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

--- Comment #7 from Darin Adler <darin at apple.com> ---
Comment on attachment 254883
  --> https://bugs.webkit.org/attachment.cgi?id=254883
Patch

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

> Source/WebCore/Modules/mediasession/MediaSession.cpp:99
> +    HashSet<HTMLMediaElement*> activeParticipatingElementsCopy = m_activeParticipatingElements;
> +
> +    for (auto* element : activeParticipatingElementsCopy) {

This pattern almost always leads to serious bugs. Once you have copied the HTMLMediaElement set, elements from the set could be deleted as a side effect of the operations below, and then you could use an element pointer of a deleted object.

One technique is removing each element from the set as we iterate instead of using a for loop, using the HashSet::takeAny function. Then also making sure that when an element is removed from the “real” set it’s also removed from the set currently being iterated. That pattern is used in DisplayRefreshMonitor::displayDidRefresh.

Another technique is to use Vector<RefPtr<HTMLMediaElement>> instead of HashSet<MediaElement*> for the elements we are iterating. That guarantees the elements are not deallocated, but we might not want to toggle the state of an element that has been removed from the document tree, for example.

I know we have run into the same problem elsewhere and solved it multiple ways. But this code is not safe.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150615/1d669c70/attachment.html>