Stop using CheckedPtr with CoreAudioCaptureSource

https://bugs.webkit.org/show_bug.cgi?id=266714 Reviewed by Youenn Fablet. Stop using CheckedPtr with CoreAudioCaptureSource. Use ThreadSafeWeakPtr instead to generate more actionable crashes and since the class is ThreadSafeRefCounted. * Source/WTF/wtf/ThreadSafeWeakHashSet.h: * Source/WTF/wtf/ThreadSafeWeakPtr.h: (WTF::ThreadSafeWeakPtr::ThreadSafeWeakPtr): * Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.cpp: (WebCore::BaseAudioSharedUnit::addClient): (WebCore::BaseAudioSharedUnit::removeClient): (WebCore::BaseAudioSharedUnit::forEachClient const): (WebCore::BaseAudioSharedUnit::audioSamplesAvailable): * Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.h: (WebCore::BaseAudioSharedUnit::hasClients const): * Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.h: Canonical link: https://commits.webkit.org/272420@main
WebKit · Dec 21, 2023 · de025ed · de025ed
1 parent a7e2900
commit de025ed
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 15 deletions.
diff --git a/Source/WTF/wtf/ThreadSafeWeakHashSet.h b/Source/WTF/wtf/ThreadSafeWeakHashSet.h
@@ -165,6 +165,18 @@ class ThreadSafeWeakHashSet final {
         return strongReferences;
     }
 
+    Vector<ThreadSafeWeakPtr<T>> weakValues() const
+    {
+        Vector<ThreadSafeWeakPtr<T>> weakReferences;
+        {
+            Locker locker { m_lock };
+            weakReferences = WTF::map(m_map, [](auto& pair) {
+                return ThreadSafeWeakPtr { *pair.value, *pair.key };
+            });
+        }
+        return weakReferences;
+    }
+
     template<typename Functor>
     void forEach(const Functor& callback) const
     {

diff --git a/Source/WTF/wtf/ThreadSafeWeakPtr.h b/Source/WTF/wtf/ThreadSafeWeakPtr.h
@@ -217,6 +217,10 @@ class ThreadSafeWeakPtr {
         : m_controlBlock(std::exchange(other.m_controlBlock, nullptr))
         , m_objectOfCorrectType(std::exchange(other.m_objectOfCorrectType, nullptr)) { }
 
+    ThreadSafeWeakPtr(ThreadSafeWeakPtrControlBlock& controlBlock, const T& objectOfCorrectType)
+        : m_controlBlock(&controlBlock)
+        , m_objectOfCorrectType(&objectOfCorrectType) { }
+
     ThreadSafeWeakPtr& operator=(ThreadSafeWeakPtr&& other)
     {
         m_controlBlock = std::exchange(other.m_controlBlock, nullptr);

diff --git a/Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.cpp b/Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.cpp
@@ -52,17 +52,17 @@ BaseAudioSharedUnit::~BaseAudioSharedUnit()
 void BaseAudioSharedUnit::addClient(CoreAudioCaptureSource& client)
 {
     ASSERT(isMainThread());
-    m_clients.add(&client);
+    m_clients.add(client);
     Locker locker { m_audioThreadClientsLock };
-    m_audioThreadClients = copyToVector(m_clients);
+    m_audioThreadClients = m_clients.weakValues();
 }
 
 void BaseAudioSharedUnit::removeClient(CoreAudioCaptureSource& client)
 {
     ASSERT(isMainThread());
-    m_clients.remove(&client);
+    m_clients.remove(client);
     Locker locker { m_audioThreadClientsLock };
-    m_audioThreadClients = copyToVector(m_clients);
+    m_audioThreadClients = m_clients.weakValues();
 }
 
 void BaseAudioSharedUnit::clearClients()
@@ -76,12 +76,7 @@ void BaseAudioSharedUnit::clearClients()
 void BaseAudioSharedUnit::forEachClient(const Function<void(CoreAudioCaptureSource&)>& apply) const
 {
     ASSERT(isMainThread());
-    for (auto& client : copyToVector(m_clients)) {
-        // Make sure the client has not been destroyed.
-        if (!m_clients.contains(client.get()))
-            continue;
-        apply(*client);
-    }
+    m_clients.forEach(apply);
 }
 
 const static OSStatus lowPriorityError1 = 560557684;
@@ -307,7 +302,10 @@ void BaseAudioSharedUnit::audioSamplesAvailable(const MediaTime& time, const Pla
     // For performance reasons, we forbid heap allocations while doing rendering on the capture audio thread.
     ForbidMallocUseForCurrentThreadScope forbidMallocUse;
 
-    for (auto& client : m_audioThreadClients) {
+    for (auto& weakClient : m_audioThreadClients) {
+        RefPtr client = weakClient.get();
+        if (!client)
+            continue;
         if (client->isProducingData())
             client->audioSamplesAvailable(time, data, description, numberOfFrames);
     }

diff --git a/Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.h b/Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.h
@@ -84,7 +84,7 @@ class BaseAudioSharedUnit : public RealtimeMediaSourceCenter::Observer {
 
     void whenAudioCaptureUnitIsNotRunning(Function<void()>&&);
     bool isRenderingAudio() const { return m_isRenderingAudio; }
-    bool hasClients() const { return !m_clients.isEmpty(); }
+    bool hasClients() const { return !m_clients.isEmptyIgnoringNullReferences(); }
 
     const String& persistentIDForTesting() const { return m_capturingDevice ? m_capturingDevice->first : emptyString(); }
 
@@ -138,8 +138,8 @@ class BaseAudioSharedUnit : public RealtimeMediaSourceCenter::Observer {
     uint32_t m_outputDeviceID { 0 };
     std::optional<std::pair<String, uint32_t>> m_capturingDevice;
 
-    HashSet<CheckedPtr<CoreAudioCaptureSource>> m_clients;
-    Vector<CheckedPtr<CoreAudioCaptureSource>> m_audioThreadClients WTF_GUARDED_BY_LOCK(m_audioThreadClientsLock);
+    ThreadSafeWeakHashSet<CoreAudioCaptureSource> m_clients;
+    Vector<ThreadSafeWeakPtr<CoreAudioCaptureSource>> m_audioThreadClients WTF_GUARDED_BY_LOCK(m_audioThreadClientsLock);
     Lock m_audioThreadClientsLock;
 
     bool m_isCapturingWithDefaultMicrophone { false };

diff --git a/Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.h b/Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.h
@@ -51,7 +51,7 @@ class BaseAudioSharedUnit;
 class CaptureDeviceInfo;
 class WebAudioSourceProviderAVFObjC;
 
-class CoreAudioCaptureSource : public RealtimeMediaSource, public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<CoreAudioCaptureSource, WTF::DestructionThread::MainRunLoop>, public CanMakeCheckedPtr {
+class CoreAudioCaptureSource : public RealtimeMediaSource, public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<CoreAudioCaptureSource, WTF::DestructionThread::MainRunLoop> {
 public:
     WEBCORE_EXPORT static CaptureSourceOrError create(String&& deviceID, MediaDeviceHashSalts&&, const MediaConstraints*, PageIdentifier);
     static CaptureSourceOrError createForTesting(String&& deviceID, AtomString&& label, MediaDeviceHashSalts&&, const MediaConstraints*, BaseAudioSharedUnit& overrideUnit, PageIdentifier);