From 272b4e576f50cea4aeeb00c02fd69d8c1ad0b246 Mon Sep 17 00:00:00 2001 From: Vincent Lucas <chenzo@jitsi.org> Date: Tue, 1 Oct 2013 22:46:33 +0200 Subject: [PATCH] Corrects deadloack between the CoreAudio AudioDeviceStop function and the stream processing callback. --- .../maccoreaudio/MacCoreaudioStream.java | 36 +++++++- .../renderer/audio/MacCoreaudioRenderer.java | 92 ++++++++++++++----- 2 files changed, 98 insertions(+), 30 deletions(-) diff --git a/src/org/jitsi/impl/neomedia/jmfext/media/protocol/maccoreaudio/MacCoreaudioStream.java b/src/org/jitsi/impl/neomedia/jmfext/media/protocol/maccoreaudio/MacCoreaudioStream.java index 40711edb..c2777d1d 100644 --- a/src/org/jitsi/impl/neomedia/jmfext/media/protocol/maccoreaudio/MacCoreaudioStream.java +++ b/src/org/jitsi/impl/neomedia/jmfext/media/protocol/maccoreaudio/MacCoreaudioStream.java @@ -8,6 +8,7 @@ import java.io.*; import java.util.*; +import java.util.concurrent.locks.*; import javax.media.*; import javax.media.control.*; @@ -96,6 +97,12 @@ public class MacCoreaudioStream */ private final GainControl gainControl; + /** + * Locked when currently stopping the stream. Prevents deadlock between the + * CaoreAudio callback and the AudioDeviceStop function. + */ + private Lock stopLock = new ReentrantLock(); + private final MacCoreaudioSystem.UpdateAvailableDeviceListListener updateAvailableDeviceListListener = new MacCoreaudioSystem.UpdateAvailableDeviceListListener() @@ -379,7 +386,16 @@ public void stop() { if(stream != 0 && deviceUID != null) { - MacCoreAudioDevice.stopStream(deviceUID, stream); + stopLock.lock(); + try + { + MacCoreAudioDevice.stopStream(deviceUID, stream); + } + finally + { + stopLock.unlock(); + } + stream = 0; this.fullBufferList.clear(); this.freeBufferList.clear(); @@ -421,12 +437,22 @@ public void readInput(byte[] buffer, int bufferLength) this.fullBufferList.add(this.buffer); this.buffer = null; nbBufferData = 0; - synchronized(startStopMutex) + if(stopLock.tryLock()) { - startStopMutex.notify(); - if(this.freeBufferList.size() > 0) + try + { + synchronized(startStopMutex) + { + startStopMutex.notify(); + if(this.freeBufferList.size() > 0) + { + this.buffer = this.freeBufferList.remove(0); + } + } + } + finally { - this.buffer = this.freeBufferList.remove(0); + stopLock.unlock(); } } diff --git a/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/MacCoreaudioRenderer.java b/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/MacCoreaudioRenderer.java index 1657c645..30c381c1 100644 --- a/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/MacCoreaudioRenderer.java +++ b/src/org/jitsi/impl/neomedia/jmfext/media/renderer/audio/MacCoreaudioRenderer.java @@ -9,6 +9,7 @@ import java.beans.*; import java.lang.reflect.*; import java.util.*; +import java.util.concurrent.locks.*; import javax.media.*; import javax.media.format.*; @@ -63,8 +64,17 @@ public class MacCoreaudioRenderer */ private int nbBufferData = 0; + /** + * Indicates when we start to close the stream. + */ private boolean isStopping = false; + /** + * Locked when currently stopping the stream. Prevents deadlock between the + * CaoreAudio callback and the AudioDeviceStop function. + */ + private Lock stopLock = new ReentrantLock(); + /** * The constant which represents an empty array with * <tt>Format</tt> element type. Explicitly defined in order to @@ -471,6 +481,7 @@ public void stop() long startTime = System.currentTimeMillis(); long currentTime = startTime; long startNbData = nbBufferData; + // Wait at most 500 ms to render the already received data. while(nbBufferData > 0 && (currentTime - startTime) < timeout) { @@ -490,7 +501,16 @@ public void stop() } } - MacCoreAudioDevice.stopStream(deviceUID, stream); + stopLock.lock(); + try + { + MacCoreAudioDevice.stopStream(deviceUID, stream); + } + finally + { + stopLock.unlock(); + } + stream = 0; buffer = null; nbBufferData = 0; @@ -508,38 +528,54 @@ public void stop() */ public void writeOutput(byte[] buffer, int bufferLength) { - synchronized(startStopMutex) + // If the stop function has been called, then skip the synchronize which + // can lead to a deadlock because the AudioDeviceStop native function + // waits for this callback to end. + if(stopLock.tryLock()) { - updateBufferLength(bufferLength); - - int i = 0; - int length = nbBufferData; - if(bufferLength < length) + try { - length = bufferLength; - } + synchronized(startStopMutex) + { + updateBufferLength(bufferLength); - System.arraycopy(this.buffer, 0, buffer, 0, length); + int i = 0; + int length = nbBufferData; + if(bufferLength < length) + { + length = bufferLength; + } - // Fiils the end of the buffer with silence. - if(length < bufferLength) - { - Arrays.fill(buffer, length, bufferLength, (byte) 0); - } + System.arraycopy(this.buffer, 0, buffer, 0, length); + // Fills the end of the buffer with silence. + if(length < bufferLength) + { + Arrays.fill(buffer, length, bufferLength, (byte) 0); + } - nbBufferData -= length; - if(nbBufferData > 0) - { - System.arraycopy( - this.buffer, length, this.buffer, 0, nbBufferData); + + nbBufferData -= length; + if(nbBufferData > 0) + { + System.arraycopy( + this.buffer, + length, + this.buffer, + 0, + nbBufferData); + } + // If the stop process is waiting, notifies that every + // sample has been consummed (nbBufferData == 0). + else + { + startStopMutex.notify(); + } + } } - // If the stop process is waiting, notifies it that every sample - // has been consummed. - // (nbBufferData == 0) - else + finally { - startStopMutex.notify(); + stopLock.unlock(); } } } @@ -567,6 +603,12 @@ private boolean updateDeviceUID() return false; } + /** + * Increases the buffer length if necessary: if the new legnth is greater + * than the current buffer length. + * + * @param newLength The new length requested. + */ private void updateBufferLength(int newLength) { synchronized(startStopMutex) -- GitLab