Changeset 215546 in webkit

Timestamp:

Apr 19, 2017 5:53:37 PM (7 years ago)

Author:

eric.carlson@apple.com

Message:

Another deadlock in CoreAudioCaptureSource
​https://bugs.webkit.org/show_bug.cgi?id=171001

Fix another regression introduced by r215201, plus make changes suggested
in the review of 170771.

Reviewed by Youenn Fablet.

• platform/mediastream/mac/CoreAudioCaptureSource.cpp:

(WebCore::CoreAudioCaptureSource::configureSpeakerProc): Assert if the lock is no held.
(WebCore::CoreAudioCaptureSource::provideSpeakerData): Don't reset the buffer.
(WebCore::CoreAudioCaptureSource::processMicrophoneSamples): Take the state lock. Don't
reset the buffer. No more microphone callbacks.
(WebCore::CoreAudioCaptureSource::stopProducingData): Return early if the io unit isn't
running. Drop the lock before calling setMuted to avoid another deadlock.
(WebCore::CoreAudioCaptureSource::addMicrophoneDataConsumer): Deleted.
(WebCore::CoreAudioCaptureSource::removeMicrophoneDataConsumer): Deleted.

• platform/mediastream/mac/CoreAudioCaptureSource.h:

Location:

trunk/Source/WebCore

Files:

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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-04-19  Eric Carlson  <eric.carlson@apple.com>
+
+        Another deadlock in CoreAudioCaptureSource
+        https://bugs.webkit.org/show_bug.cgi?id=171001
+
+        Fix another regression introduced by r215201, plus make changes suggested
+        in the review of 170771.
+
+        Reviewed by Youenn Fablet.
+
+        * platform/mediastream/mac/CoreAudioCaptureSource.cpp:
+        (WebCore::CoreAudioCaptureSource::configureSpeakerProc): Assert if the lock is no held.
+        (WebCore::CoreAudioCaptureSource::provideSpeakerData): Don't reset the buffer.
+        (WebCore::CoreAudioCaptureSource::processMicrophoneSamples): Take the state lock. Don't
+        reset the buffer. No more microphone callbacks.
+        (WebCore::CoreAudioCaptureSource::stopProducingData): Return early if the io unit isn't
+        running. Drop the lock before calling setMuted to avoid another deadlock.
+        (WebCore::CoreAudioCaptureSource::addMicrophoneDataConsumer): Deleted.
+        (WebCore::CoreAudioCaptureSource::removeMicrophoneDataConsumer): Deleted.
+        * platform/mediastream/mac/CoreAudioCaptureSource.h:
+
 2017-04-19  Anders Carlsson  <andersca@apple.com>
 

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

@@ -171 +171 @@
 OSStatus CoreAudioCaptureSource::configureSpeakerProc()
 {
+    ASSERT(m_internalStateLock.isHeld());
+
     AURenderCallbackStruct callback = { speakerCallback, this };
     auto err = AudioUnitSetProperty(m_ioUnit, kAudioUnitProperty_SetRenderCallback, kAudioUnitScope_Global, outputBus, &callback, sizeof(callback));
@@ -192 +194 @@
     return err;
 }
-
-uint64_t CoreAudioCaptureSource::addMicrophoneDataConsumer(MicrophoneDataCallback&& callback)
-{
-    std::lock_guard<Lock> lock(m_pendingSourceQueueLock);
-    m_microphoneDataCallbacks.add(++m_nextMicrophoneDataCallbackID, callback);
-
-    return m_nextMicrophoneDataCallbackID;
-}
-
-void CoreAudioCaptureSource::removeMicrophoneDataConsumer(uint64_t callbackID)
-{
-    std::lock_guard<Lock> lock(m_pendingSourceQueueLock);
-    m_microphoneDataCallbacks.remove(callbackID);
-}    
 
 void CoreAudioCaptureSource::addEchoCancellationSource(AudioSampleDataSource& source)
@@ -265 +253 @@
     uint64_t sampleTime = timeStamp.mSampleTime;
     checkTimestamps(timeStamp, sampleTime, adjustedHostTime);
-
-    m_speakerSampleBuffer->reset();
     m_speakerSampleBuffer->setTimes(adjustedHostTime, sampleTime);
 
@@ -292 +278 @@
 OSStatus CoreAudioCaptureSource::processMicrophoneSamples(AudioUnitRenderActionFlags& ioActionFlags, const AudioTimeStamp& timeStamp, UInt32 inBusNumber, UInt32 inNumberFrames, AudioBufferList* /*ioData*/)
 {
+    std::lock_guard<Lock> lock(m_internalStateLock);
+
     ++m_microphoneProcsCalled;
 
     // Pull through the vpio unit to our mic buffer.
-    m_microphoneSampleBuffer->reset();
     AudioBufferList& bufferList = m_microphoneSampleBuffer->bufferList();
     auto err = AudioUnitRender(m_ioUnit, &ioActionFlags, &timeStamp, inBusNumber, inNumberFrames, &bufferList);
@@ -307 +294 @@
     checkTimestamps(timeStamp, sampleTime, adjustedHostTime);
     m_latestMicTimeStamp = sampleTime;
-
     m_microphoneSampleBuffer->setTimes(adjustedHostTime, sampleTime);
 
     audioSamplesAvailable(MediaTime(sampleTime, m_microphoneProcFormat.sampleRate()), m_microphoneSampleBuffer->bufferList(), m_microphoneProcFormat, inNumberFrames);
-
-    if (m_microphoneDataCallbacks.isEmpty())
-        return 0;
-
-    for (auto& callback : m_microphoneDataCallbacks.values())
-        callback(MediaTime(sampleTime, m_microphoneProcFormat.sampleRate()), m_microphoneSampleBuffer->bufferList(), m_microphoneProcFormat, inNumberFrames);
 
     return noErr;
@@ -468 +448 @@
 void CoreAudioCaptureSource::stopProducingData()
 {
-    std::lock_guard<Lock> lock(m_internalStateLock);
-
-    if (!m_ioUnit)
+    if (!m_ioUnit || !m_ioUnitStarted)
         return;
 
-    auto err = AudioOutputUnitStop(m_ioUnit);
-    if (err) {
-        LOG(Media, "CoreAudioCaptureSource::stop(%p) AudioOutputUnitStop failed with error %d (%.4s)", this, (int)err, (char*)&err);
-        return;
-    }
-    m_ioUnitStarted = false;
+    {
+        std::lock_guard<Lock> lock(m_internalStateLock);
+
+        auto err = AudioOutputUnitStop(m_ioUnit);
+        if (err) {
+            LOG(Media, "CoreAudioCaptureSource::stop(%p) AudioOutputUnitStop failed with error %d (%.4s)", this, (int)err, (char*)&err);
+            return;
+        }
+        m_ioUnitStarted = false;
+    }
+
     setMuted(true);
 }

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

@@ -116 +116 @@
     uint64_t m_latestMicTimeStamp { 0 };
 
-    HashMap<uint64_t, MicrophoneDataCallback> m_microphoneDataCallbacks;
-    uint64_t m_nextMicrophoneDataCallbackID { 0 };
-
     CAAudioStreamDescription m_speakerProcFormat;
     RefPtr<AudioSampleBufferList> m_speakerSampleBuffer;