From 0bc7cf94651c9174462505a2a6fbd783f728bcf0 Mon Sep 17 00:00:00 2001 From: Phil Burk Date: Sat, 27 Aug 2016 13:53:10 -0700 Subject: [PATCH] pa_mac_core_blocking: fix hang when running stream stopped One thread was filling the buffer while another thread was waiting for it to be empty. Add thread safety warnings. Fix ticket #252 --- include/portaudio.h | 10 +- src/hostapi/coreaudio/pa_mac_core.c | 36 ++-- src/hostapi/coreaudio/pa_mac_core_blocking.c | 62 ++++--- src/hostapi/coreaudio/pa_mac_core_blocking.h | 2 +- test/patest_write_stop_hang_illegal.c | 166 +++++++++++++++++++ 5 files changed, 235 insertions(+), 41 deletions(-) create mode 100644 test/patest_write_stop_hang_illegal.c diff --git a/include/portaudio.h b/include/portaudio.h index 3bb3b31..42e4d6d 100644 --- a/include/portaudio.h +++ b/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. @@ -1149,9 +1149,9 @@ PaError Pa_ReadStream( PaStream* stream, will want to match this parameter to the framesPerBuffer parameter used when opening the stream. - @return On success PaNoError will be returned, or paOutputUnderflowed if - additional output data was inserted after the previous call and before this - call. + @return On success PaNoError will be returned, + or paOutputUnderflowed if additional output data was inserted after the + previous call and before this call. */ PaError Pa_WriteStream( PaStream* stream, const void *buffer, diff --git a/src/hostapi/coreaudio/pa_mac_core.c b/src/hostapi/coreaudio/pa_mac_core.c index 4b7b43c..896ee64 100644 --- a/src/hostapi/coreaudio/pa_mac_core.c +++ b/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,30 @@ 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 while we are waiting for it to drain. + stream->state = STOPPING; + + VDBUG( ("Waiting for BLIO.\n") ); + paErr = waitUntilBlioWriteBufferIsEmpty( &stream->blio, stream->sampleRate ); + VDBUG( ( "waitUntilBlioWriteBufferIsEmpty returned %d\n", paErr ) ); + + return FinishStoppingStream( stream ); +} + +/* Immediately stop the stream. */ static PaError AbortStream( PaStream *s ) { - VVDBUG(("AbortStream()->StopStream()\n")); - VDBUG( ( "Aborting stream.\n" ) ); - /* We have nothing faster than StopStream. */ - return StopStream(s); + PaMacCoreStream *stream = (PaMacCoreStream*)s; + VDBUG( ( "AbortStream()\n" ) ); + stream->state = STOPPING; + return FinishStoppingStream( stream ); } diff --git a/src/hostapi/coreaudio/pa_mac_core_blocking.c b/src/hostapi/coreaudio/pa_mac_core_blocking.c index 92c3251..959b765 100644 --- a/src/hostapi/coreaudio/pa_mac_core_blocking.c +++ b/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,34 +565,46 @@ PaError WriteStream( PaStream* stream, #endif } - /* Report either paNoError or paOutputUnderflowed. */ - /* may also want to report other errors, but this is non-standard. */ - /* FIXME should not clobber ret, use if(blio->statusFlags & paInputOverflow) */ - ret = blio->statusFlags & paOutputUnderflow; - - /* report underflow only once: */ - if( ret ) { - OSAtomicAnd32( (uint32_t)(~paOutputUnderflow), &blio->statusFlags ); - ret = paOutputUnderflowed; + if ( macStream->state == STOPPING ) + { + ret = paInternalError; + } + else if (ret == paNoError ) + { + /* Test for underflow. */ + ret = blio->statusFlags & paOutputUnderflow; + + /* report underflow only once: */ + if( ret ) { + 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 ) { + PaError result = paNoError; if( blio->outputRingBuffer.buffer ) { - /* FIXME loop until PaUtil_GetRingBufferReadAvailable==0 */ - 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 ); - } + int timeout = 5; // don't wait forever + ring_buffer_size_t framesInBuffer = PaUtil_GetRingBufferReadAvailable( &blio->outputRingBuffer ); + while( framesInBuffer > 0 && timeout-- > 0 ) { + long msecEstimated = 1 + (long)( 1000.0 * framesInBuffer / sampleRate); + VDBUG(( "waitUntilBlioWriteBufferIsFlushed: framesInBuffer = %d, msecEstimated = %ld\n", framesInBuffer, msecEstimated )); + Pa_Sleep( msecEstimated ); + framesInBuffer = PaUtil_GetRingBufferReadAvailable( &blio->outputRingBuffer ); + } + if( framesInBuffer > 0 ) + { + result = paTimedOut; + } } + return result; } diff --git a/src/hostapi/coreaudio/pa_mac_core_blocking.h b/src/hostapi/coreaudio/pa_mac_core_blocking.h index c669e0b..8a9707b 100644 --- a/src/hostapi/coreaudio/pa_mac_core_blocking.h +++ b/src/hostapi/coreaudio/pa_mac_core_blocking.h @@ -128,6 +128,6 @@ int BlioCallback( PaStreamCallbackFlags statusFlags, void *userData ); -void waitUntilBlioWriteBufferIsFlushed( PaMacBlio *blio ); +PaError waitUntilBlioWriteBufferIsEmpty( PaMacBlio *blio, double sampleRate ); #endif /*PA_MAC_CORE_BLOCKING_H_*/ diff --git a/test/patest_write_stop_hang_illegal.c b/test/patest_write_stop_hang_illegal.c new file mode 100644 index 0000000..35f73a0 --- /dev/null +++ b/test/patest_write_stop_hang_illegal.c @@ -0,0 +1,166 @@ +/** @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 +#include +#include +/* pthread may only be available on Mac and Linux. */ +#include +#include "portaudio.h" + +#define SAMPLE_RATE (44100) +#define FRAMES_PER_BUFFER (4096) + +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++) + { + time = Pa_GetStreamTime( stream ); + printf("Stream time = %f\n", time); + fflush(stdout); + Pa_Sleep(100); + } + 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; +} -- 2.43.0