Changeset 247381 in webkit

Timestamp:

Jul 11, 2019 9:22:36 PM (5 years ago)

Author:

youenn@apple.com

Message:

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/mediastream/mac/CoreAudioCaptureSource.cpp (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

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

@@ -129 +129 @@
     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 };
@@ -197 +200 @@
 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);
+    }
 }
 
@@ -510 +529 @@
         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;
 }
@@ -657 +676 @@
 {
     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();