Race condition in StreamProcessingCallback

[darksylinc]

Describe the bug

I have not encountered this bug, but during my analysis of #15 I noticed this race condition.

StreamProcessingCallback performs the following:

if( framesProcessed == 0 && stream->doStop ) /* StopStream was called */
{
  // ...
  PaUnixThreading_EXIT( paNoError );
  return;
}
else if( framesProcessed == 0 && !(stream->doAbort || stream->doStop) ) /* if AbortStream or StopStream weren't called, stop from the cb */
{
  // ...
  PaUnixThreading_EXIT( paNoError );
  return;
}

Sequentially, it makes sense: If framesProcessed == 0 it means the callback has requested to stop/abort.

That means enqueue() hasn't been called.

However the problem is that the following execution is possible (assume framesProcessed = 0):

/* Thread A */ if( framesProcessed == 0 && stream->doStop ) // stream->doStop = false
/* Thread B */ stream->doStop = true
/* Thread A */ else if( framesProcessed == 0 && !(stream->doAbort || stream->doStop) )

The result is that neither branch is entered, because stream->doStop changed in the middle. Depending on how the compiler compiles this code (and debug vs optimized will certainly be different) this can cause a race condition.

Since enqueue() hasn't been called and the code entered neither of the branches, it will iterate again.

Then it will enter sem_wait( &stream->outputSem ); and never wake up again; because enqueue was not called.

To Reproduce

I don't have a repro because this is theoretical. But it should be possible to create a tight loop or play/pause that tries to create the necessary conditions.

Solution

const SLboolean bDoStop = stream->doStop;
const SLboolean bDoAbort = stream->doAbort;

PaUtil_FullMemoryBarrier();

if( framesProcessed == 0 && bDoStop ) /* StopStream was called */
{
  // ...
}
else if( framesProcessed == 0 && !(bDoStop || bDoAbort) )
{
}

Expected behavior

Actual behavior

Desktop (please complete the following information):

• OS: Android
• OS Version Any
• PortAudio version:

Additional context