Merge branch 'ticket_252_nohang' into 'master'

pa_mac_core_blocking: fix hang when running stream stopped (fixed merge)

Merged-on: https://assembla.com/code/portaudio/git/merge_requests/3829453

include/portaudio.h

@@ -934,7 +934,7 @@ PaError Pa_CloseStream( PaStream *stream );
  (ie once a call to Pa_StopStream() will not block).
  A stream will become inactive after the stream callback returns non-zero,
  or when Pa_StopStream or Pa_AbortStream is called. For a stream providing audio
- output, if the stream callback returns paComplete, or Pa_StopStream is called,
+ output, if the stream callback returns paComplete, or Pa_StopStream() is called,
  the stream finished callback will not be called until all generated sample data
  has been played.
  
@@ -1132,7 +1132,7 @@ PaError Pa_ReadStream( PaStream* stream,
 
 
 /** Write samples to an output stream. This function doesn't return until the
- entire buffer has been consumed - this may involve waiting for the operating
+ entire buffer has been written - this may involve waiting for the operating
  system to consume the data.
 
  @param stream A pointer to an open stream previously created with Pa_OpenStream.

src/hostapi/coreaudio/pa_mac_core.c

@@ -2724,18 +2724,10 @@ static ComponentResult BlockWhileAudioUnitIsRunning( AudioUnit audioUnit, AudioU
     return noErr;
 }
 
-static PaError StopStream( PaStream *s )
+static PaError FinishStoppingStream( PaMacCoreStream *stream )
 {
-    PaMacCoreStream *stream = (PaMacCoreStream*)s;
     OSStatus result = noErr;
     PaError paErr;
-    VVDBUG(("StopStream()\n"));
-
-    VDBUG( ("Waiting for BLIO.\n") );
-    waitUntilBlioWriteBufferIsFlushed( &stream->blio );
-    VDBUG( ( "Stopping stream.\n" ) );
-
-    stream->state = STOPPING;
 
 #define ERR_WRAP(mac_err) do { result = mac_err ; if ( result != noErr ) return ERR(result) ; } while(0)
     /* -- stop and reset -- */
@@ -2787,12 +2779,34 @@ static PaError StopStream( PaStream *s )
 #undef ERR_WRAP
 }
 
+/* Block until buffer is empty then stop the stream. */
+static PaError StopStream( PaStream *s )
+{
+    PaError paErr;
+    PaMacCoreStream *stream = (PaMacCoreStream*)s;
+    VVDBUG(("StopStream()\n"));
+
+    /* Tell WriteStream to stop filling the buffer. */
+    stream->state = STOPPING;
+
+    if( stream->userOutChan > 0 ) /* Does this stream do output? */
+    {
+        size_t maxHostFrames = MAX( stream->inputFramesPerBuffer, stream->outputFramesPerBuffer );
+        VDBUG( ("Waiting for write buffer to be drained.\n") );
+        paErr = waitUntilBlioWriteBufferIsEmpty( &stream->blio, stream->sampleRate,
+                                                maxHostFrames );
+        VDBUG( ( "waitUntilBlioWriteBufferIsEmpty returned %d\n", paErr ) );
+    }
+    return FinishStoppingStream( stream );
+}
+
+/* Immediately stop the stream. */
 static PaError AbortStream( PaStream *s )
 {
-    VVDBUG(("AbortStream()->StopStream()\n"));
+    PaMacCoreStream *stream = (PaMacCoreStream*)s;
-    VDBUG( ( "Aborting stream.\n" ) );
+    VDBUG( ( "AbortStream()\n" ) );
-    /* We have nothing faster than StopStream. */
+    stream->state = STOPPING;
-    return StopStream(s);
+    return FinishStoppingStream( stream );
 }
 
 

src/hostapi/coreaudio/pa_mac_core_blocking.c

@@ -455,7 +455,7 @@ PaError ReadStream( PaStream* stream,
 #endif
           }
        } while( framesAvailable == 0 );
-       framesToTransfer = MIN( framesAvailable, framesRequested );
+       framesToTransfer = (ring_buffer_size_t) MIN( framesAvailable, framesRequested );
        framesTransferred = PaUtil_ReadRingBuffer( &blio->inputRingBuffer, (void *)cbuf, framesToTransfer );
        cbuf += framesTransferred * blio->inputSampleSizeActual * blio->inChan;
        framesRequested -= framesTransferred;
@@ -499,12 +499,13 @@ PaError WriteStream( PaStream* stream,
                             const void *buffer,
                             unsigned long framesRequested )
 {
-    PaMacBlio *blio = & ((PaMacCoreStream*)stream) -> blio;
+    PaMacCoreStream *macStream = (PaMacCoreStream*)stream;
+    PaMacBlio *blio = &macStream->blio;
     char *cbuf = (char *) buffer;
     PaError ret = paNoError;
     VVDBUG(("WriteStream()\n"));
 
-    while( framesRequested > 0 ) {
+    while( framesRequested > 0 && macStream->state != STOPPING ) {
         ring_buffer_size_t framesAvailable;
         ring_buffer_size_t framesToTransfer;
         ring_buffer_size_t framesTransferred;
@@ -534,7 +535,12 @@ PaError WriteStream( PaStream* stream,
              Pa_Sleep( PA_MAC_BLIO_BUSY_WAIT_SLEEP_INTERVAL );
 #endif
           }
-       } while( framesAvailable == 0 );
+       } while( framesAvailable == 0 && macStream->state != STOPPING );
+
+       if( macStream->state == STOPPING )
+       {
+           break;
+       }
 
        framesToTransfer = MIN( framesAvailable, framesRequested );
        framesTransferred = PaUtil_WriteRingBuffer( &blio->outputRingBuffer, (void *)cbuf, framesToTransfer );
@@ -559,36 +565,58 @@ PaError WriteStream( PaStream* stream,
 #endif
     }
 
-    /*   Report either paNoError or paOutputUnderflowed. */
+    if ( macStream->state == STOPPING )
-    /*   may also want to report other errors, but this is non-standard. */
+    {
-    /* FIXME should not clobber ret, use if(blio->statusFlags & paInputOverflow) */
+        ret = paInternalError;
-    ret = blio->statusFlags & paOutputUnderflow;
+    }
+    else if (ret == paNoError )
+    {
+        /*   Test for underflow. */
+        ret = blio->statusFlags & paOutputUnderflow;
 
-    /* report underflow only once: */
+        /* report underflow only once: */
-    if( ret ) {
+        if( ret )
-      OSAtomicAnd32( (uint32_t)(~paOutputUnderflow), &blio->statusFlags );
+        {
-      ret = paOutputUnderflowed;
+            OSAtomicAnd32( (uint32_t)(~paOutputUnderflow), &blio->statusFlags );
+            ret = paOutputUnderflowed;
+        }
     }
 
     return ret;
 }
 
 /*
- *
+ * Wait until the data in the buffer has finished playing.
  */
-void waitUntilBlioWriteBufferIsFlushed( PaMacBlio *blio )
+PaError waitUntilBlioWriteBufferIsEmpty( PaMacBlio *blio, double sampleRate,
+                                        size_t framesPerBuffer )
 {
+    PaError result = paNoError;
     if( blio->outputRingBuffer.buffer ) {
-        /* FIXME loop until PaUtil_GetRingBufferReadAvailable==0 */
+        ring_buffer_size_t framesLeft = PaUtil_GetRingBufferReadAvailable( &blio->outputRingBuffer );
-       ring_buffer_size_t framesAvailable = PaUtil_GetRingBufferWriteAvailable( &blio->outputRingBuffer );
-       while( framesAvailable != blio->outputRingBuffer.bufferSize ) {
-          if( framesAvailable == 0 )
-             Pa_Sleep( PA_MAC_BLIO_BUSY_WAIT_SLEEP_INTERVAL );
-          framesAvailable = PaUtil_GetRingBufferWriteAvailable( &blio->outputRingBuffer );
-       }
-    }
-}
 
+        /* Calculate when we should give up waiting. To be safe wait for two extra periods. */
+        PaTime now = PaUtil_GetTime();
+        PaTime startTime = now;
+        PaTime timeoutTime = startTime + (framesLeft + (2 * framesPerBuffer)) / sampleRate;
+
+        long msecPerBuffer = 1 + (long)( 1000.0 * framesPerBuffer / sampleRate);
+        while( framesLeft > 0 && now < timeoutTime ) {
+            VDBUG(( "waitUntilBlioWriteBufferIsFlushed: framesLeft = %d, framesPerBuffer = %ld\n",
+                  framesLeft, framesPerBuffer ));
+            Pa_Sleep( msecPerBuffer );
+            framesLeft = PaUtil_GetRingBufferReadAvailable( &blio->outputRingBuffer );
+            now = PaUtil_GetTime();
+        }
+
+        if( framesLeft > 0 )
+        {
+            VDBUG(( "waitUntilBlioWriteBufferIsFlushed: TIMED OUT - framesLeft = %d\n", framesLeft ));
+            result = paTimedOut;
+        }
+    }
+    return result;
+}
 
 signed long GetStreamReadAvailable( PaStream* stream )
 {

src/hostapi/coreaudio/pa_mac_core_blocking.h

@@ -128,6 +128,7 @@ int BlioCallback(
         PaStreamCallbackFlags statusFlags,
         void *userData );
 
-void waitUntilBlioWriteBufferIsFlushed( PaMacBlio *blio );
+PaError waitUntilBlioWriteBufferIsEmpty( PaMacBlio *blio, double sampleRate,
+                                        size_t framesPerBuffer );
 
 #endif /*PA_MAC_CORE_BLOCKING_H_*/

test/patest_write_stop_hang_illegal.c

@@ -0,0 +1,168 @@
+/** @file patest_write_stop_threads.c
+	@brief Call Pa_StopStream() from another thread to see if PortAudio hangs.
+	@author Bjorn Roche of XO Audio (www.xoaudio.com)
+	@author Ross Bencina
+	@author Phil Burk
+*/
+/*
+ * $Id$
+ *
+ * This program uses the PortAudio Portable Audio Library.
+ * For more information see: http://www.portaudio.com/
+ * Copyright (c) 1999-2000 Ross Bencina and Phil Burk
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files
+ * (the "Software"), to deal in the Software without restriction,
+ * including without limitation the rights to use, copy, modify, merge,
+ * publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+ * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF
+ * CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/*
+ * The text above constitutes the entire PortAudio license; however,
+ * the PortAudio community also makes the following non-binding requests:
+ *
+ * Any person wishing to distribute modifications to the Software is
+ * requested to send the modifications to the original developer so that
+ * they can be incorporated into the canonical version. It is also
+ * requested that these non-binding requests be included along with the
+ * license above.
+ */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <math.h>
+#include <memory.h>
+/* pthread may only be available on Mac and Linux. */
+#include <pthread.h>
+#include "portaudio.h"
+
+#define SAMPLE_RATE         (44100)
+#define FRAMES_PER_BUFFER   (2048)
+
+static float s_buffer[FRAMES_PER_BUFFER][2]; /* stereo output buffer */
+
+/**
+ * WARNING: PortAudio is NOT thread safe. DO NOT call PortAudio
+ * from multiple threads without synchronization. This test uses
+ * PA in an ILLEGAL WAY in order to try to flush out potential hang bugs.
+ * The test calls Pa_WriteStream() and Pa_StopStream() simultaneously
+ * from separate threads in order to try to cause Pa_StopStream() to hang.
+ * In the main thread we write to the stream in a loop.
+ * Then try stopping PA from another thread to see if it hangs.
+ *
+ * @note: Do not expect this test to pass. The test is only here
+ * as a debugging aid for hang bugs. Since this test uses PA in an
+ * illegal way, it may fail for reasons that are not PA bugs.
+ */
+
+/* Wait for awhile then abort the stream. */
+void *stop_thread_proc(void *arg)
+{
+    PaStream *stream = (PaStream *)arg;
+    PaTime time;
+    for (int i = 0; i < 20; i++)
+    {
+        /* ILLEGAL unsynchronised call to PA, see comment above */
+        time = Pa_GetStreamTime( stream );
+        printf("Stream time = %f\n", time);
+        fflush(stdout);
+        usleep(100 * 1000);
+    }
+    printf("Call Pa_StopStream()\n");
+    fflush(stdout);
+    /* ILLEGAL unsynchronised call to PA, see comment above */
+    PaError err = Pa_StopStream( stream );
+    printf("Pa_StopStream() returned %d\n", err);
+    fflush(stdout);
+
+    return stream;
+}
+
+int main(void);
+int main(void)
+{
+    PaStreamParameters outputParameters;
+    PaStream *stream;
+    PaError err;
+    int result;
+    pthread_t thread;
+
+    printf( "PortAudio Test: output silence and stop from another thread. SR = %d, BufSize = %d\n",
+            SAMPLE_RATE, FRAMES_PER_BUFFER);
+
+    err = Pa_Initialize();
+    if( err != paNoError ) goto error;
+
+    outputParameters.device = Pa_GetDefaultOutputDevice(); /* default output device */
+    outputParameters.channelCount = 2;       /* stereo output */
+    outputParameters.sampleFormat = paFloat32; /* 32 bit floating point output */
+    outputParameters.suggestedLatency = Pa_GetDeviceInfo( outputParameters.device )->defaultHighOutputLatency * 5;
+    outputParameters.hostApiSpecificStreamInfo = NULL;
+
+    /* open the stream */
+    err = Pa_OpenStream(
+              &stream,
+              NULL, /* no input */
+              &outputParameters,
+              SAMPLE_RATE,
+              FRAMES_PER_BUFFER,
+              paClipOff,      /* we won't output out of range samples so don't bother clipping them */
+              NULL, /* no callback, use blocking API */
+              NULL ); /* no callback, so no callback userData */
+    if( err != paNoError ) goto error;
+
+    result = pthread_create(&thread, NULL /* attributes */, stop_thread_proc, stream);
+
+    /* start the stream */
+    err = Pa_StartStream( stream );
+    if( err != paNoError ) goto error;
+
+    /* clear buffer */
+    memset( s_buffer, 0, sizeof(s_buffer) );
+
+    /* play the silent buffer many times */
+    while( Pa_IsStreamActive(stream) > 0 )
+    {
+        err = Pa_WriteStream( stream, s_buffer, FRAMES_PER_BUFFER );
+        printf("Pa_WriteStream returns %d = %s\n", err, Pa_GetErrorText( err ));
+        if( err != paNoError )
+        {
+            err = paNoError;
+            break;
+        };
+    }
+
+    printf("Try to join the thread that called Pa_StopStream().\n");
+    result = pthread_join( thread, NULL );
+    printf("pthread_join returned %d\n", result);
+
+    /* close, and terminate */
+    printf("Call Pa_CloseStream\n");
+    err = Pa_CloseStream( stream );
+    if( err != paNoError ) goto error;
+
+    Pa_Terminate();
+    printf("Test finished.\n");
+
+    return err;
+error:
+    Pa_Terminate();
+    fprintf( stderr, "An error occured while using the portaudio stream\n" );
+    fprintf( stderr, "Error number: %d\n", err );
+    fprintf( stderr, "Error message: %s\n", Pa_GetErrorText( err ) );
+    return err;
+}