Changeset 277390 in webkit

Timestamp:

May 12, 2021, 2:19:06 PM (5 years ago)

Author:

jer.noble@apple.com

Message:

TapStorage::lock freed while locked in AudioSourceProviderAVFObjC::destroyMixIfNeeded()
​https://bugs.webkit.org/show_bug.cgi?id=225706
<rdar://77719381>

Reviewed by Ryosuke Niwa.

In r275933, an update was made to protect access to TapStorage during destruction by
locking its lock inside destroyMixIfNeeded(), but if TapStorage has a refCount==1
during that teardown, the lock itself is destroyed while it is still held. Add an
explicit scoping to the lock holder, and only deref the TapStorage outside that
locking scope.

• platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm:

(WebCore::AudioSourceProviderAVFObjC::destroyMixIfNeeded):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-05-12  Jer Noble  <jer.noble@apple.com>
+
+        TapStorage::lock freed while locked in AudioSourceProviderAVFObjC::destroyMixIfNeeded()
+        https://bugs.webkit.org/show_bug.cgi?id=225706
+        <rdar://77719381>
+
+        Reviewed by Ryosuke Niwa.
+
+        In r275933, an update was made to protect access to TapStorage during destruction by
+        locking its lock inside destroyMixIfNeeded(), but if TapStorage has a refCount==1
+        during that teardown, the lock itself is destroyed while it is still held. Add an
+        explicit scoping to the lock holder, and only deref the TapStorage outside that
+        locking scope.
+
+        * platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm:
+        (WebCore::AudioSourceProviderAVFObjC::destroyMixIfNeeded):
+
 2021-05-12  Said Abou-Hallawa  <said@apple.com>
 

trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm

@@ -183 +183 @@
         return;
     ASSERT(m_tapStorage);
-    auto locker = holdLock(m_tapStorage->lock);
-    if (m_avPlayerItem)
-        [m_avPlayerItem setAudioMix:nil];
-    [m_avAudioMix setInputParameters:@[ ]];
-    m_avAudioMix.clear();
-    m_tap.clear();
-    m_tapStorage->_this = nullptr;
+    {
+        auto locker = holdLock(m_tapStorage->lock);
+        if (m_avPlayerItem)
+            [m_avPlayerItem setAudioMix:nil];
+        [m_avAudioMix setInputParameters:@[ ]];
+        m_avAudioMix.clear();
+        m_tap.clear();
+        m_tapStorage->_this = nullptr;
+        // Call unprepare, since Tap cannot call it after clear.
+        unprepare();
+        m_weakFactory.revokeAll();
+    }
     m_tapStorage = nullptr;
-    // Call unprepare, since Tap cannot call it after clear.
-    unprepare();
-    m_weakFactory.revokeAll();
 }