Protect CoreAudioSharedUnit::m_clients for accessing in different threads simultaneously
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Jul 2019 04:22:36 +0000 (04:22 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Jul 2019 04:22:36 +0000 (04:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199717

Reviewed by Eric Carlson.

Add a lock whenever accessing to m_clients.
Manual tests show that audio capture still works.

* platform/mediastream/mac/CoreAudioCaptureSource.cpp:
(WebCore::CoreAudioSharedUnit::addClient):
(WebCore::CoreAudioSharedUnit::removeClient):
(WebCore::CoreAudioSharedUnit::forEachClient const):
(WebCore::CoreAudioSharedUnit::processMicrophoneSamples):
(WebCore::CoreAudioSharedUnit::captureFailed):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247381 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp

index 76bb0fb..2c404a2 100644 (file)
@@ -1,3 +1,20 @@
+2019-07-11  Youenn Fablet  <youenn@apple.com>
+
+        Protect CoreAudioSharedUnit::m_clients for accessing in different threads simultaneously
+        https://bugs.webkit.org/show_bug.cgi?id=199717
+
+        Reviewed by Eric Carlson.
+
+        Add a lock whenever accessing to m_clients.
+        Manual tests show that audio capture still works.
+
+        * platform/mediastream/mac/CoreAudioCaptureSource.cpp:
+        (WebCore::CoreAudioSharedUnit::addClient):
+        (WebCore::CoreAudioSharedUnit::removeClient):
+        (WebCore::CoreAudioSharedUnit::forEachClient const):
+        (WebCore::CoreAudioSharedUnit::processMicrophoneSamples):
+        (WebCore::CoreAudioSharedUnit::captureFailed):
+
 2019-07-11  Chris Dumez  <cdumez@apple.com>
 
         Drop non thread-safe usage of WeakPtr in PlaybackSessionInterfaceAVKit
index c41bf7a..75b0b2f 100644 (file)
@@ -128,7 +128,10 @@ private:
     void devicesChanged();
     void captureFailed();
 
-    Vector<std::reference_wrapper<CoreAudioCaptureSource>> m_clients;
+    void forEachClient(const Function<void(CoreAudioCaptureSource&)>& apply) const;
+
+    HashSet<CoreAudioCaptureSource*> m_clients;
+    mutable RecursiveLock m_clientsLock;
 
     AudioUnit m_ioUnit { nullptr };
 
@@ -196,14 +199,30 @@ CoreAudioSharedUnit::CoreAudioSharedUnit()
 
 void CoreAudioSharedUnit::addClient(CoreAudioCaptureSource& client)
 {
-    m_clients.append(client);
+    auto locker = holdLock(m_clientsLock);
+    m_clients.add(&client);
 }
 
 void CoreAudioSharedUnit::removeClient(CoreAudioCaptureSource& client)
 {
-    m_clients.removeAllMatching([&](const auto& item) {
-        return &client == &item.get();
-    });
+    auto locker = holdLock(m_clientsLock);
+    m_clients.remove(&client);
+}
+
+void CoreAudioSharedUnit::forEachClient(const Function<void(CoreAudioCaptureSource&)>& apply) const
+{
+    Vector<CoreAudioCaptureSource*> clientsCopy;
+    {
+        auto locker = holdLock(m_clientsLock);
+        clientsCopy = copyToVector(m_clients);
+    }
+    for (auto* client : clientsCopy) {
+        auto locker = holdLock(m_clientsLock);
+        // Make sure the client has not been destroyed.
+        if (!m_clients.contains(client))
+            continue;
+        apply(*client);
+    }
 }
 
 void CoreAudioSharedUnit::setCaptureDevice(String&& persistentID, uint32_t captureDeviceID)
@@ -509,10 +528,10 @@ OSStatus CoreAudioSharedUnit::processMicrophoneSamples(AudioUnitRenderActionFlag
     if (m_volume != 1.0)
         m_microphoneSampleBuffer->applyGain(m_volume);
 
-    for (CoreAudioCaptureSource& client : m_clients) {
+    forEachClient([&](auto& client) {
         if (client.isProducingData())
             client.audioSamplesAvailable(MediaTime(sampleTime, m_microphoneProcFormat.sampleRate()), m_microphoneSampleBuffer->bufferList(), m_microphoneProcFormat, inNumberFrames);
-    }
+    });
     return noErr;
 }
 
@@ -656,11 +675,17 @@ void CoreAudioSharedUnit::verifyIsCapturing()
 void CoreAudioSharedUnit::captureFailed()
 {
     RELEASE_LOG_ERROR(WebRTC, "CoreAudioSharedUnit::captureFailed - capture failed");
-    for (CoreAudioCaptureSource& client : m_clients)
+    forEachClient([](auto& client) {
         client.captureFailed();
+    });
 
     m_producingCount = 0;
-    m_clients.clear();
+
+    {
+        auto locker = holdLock(m_clientsLock);
+        m_clients.clear();
+    }
+
     stopInternal();
     cleanupAudioUnit();
 }