Fix some potential race conditions with OpenSL

For playback, increment the ring buffer's write pointer before queueing audio,
to handle cases where the callback is invoked, advancing the read pointer,
before the write pointer is advanced.

For capture, limit the number of re-queued chunks to the number of fully read
chunks.

1 files changed, 62 insertions, 42 deletions

diff --git a/Alc/backends/opensl.c b/Alc/backends/opensl.c
index 50fe77d8..9815b390 100644
--- a/Alc/backends/opensl.c
+++ b/Alc/backends/opensl.c
@@ -206,6 +206,9 @@ static void ALCopenslPlayback_Destruct(ALCopenslPlayback* self)
     self->mEngineObj = NULL;
     self->mEngine = NULL;
 
+    ll_ringbuffer_free(self->mRing);
+    self->mRing = NULL;
+
     alsem_destroy(&self->mSem);
 
     ALCbackend_Destruct(STATIC_CAST(ALCbackend, self));
@@ -251,19 +254,16 @@ static int ALCopenslPlayback_mixerProc(void *arg)
         result = VCALL(self->mBufferQueueObj,GetInterface)(SL_IID_PLAY, &player);
         PRINTERR(result, "bufferQueue->GetInterface SL_IID_PLAY");
     }
+
+    ALCopenslPlayback_lock(self);
     if(SL_RESULT_SUCCESS != result)
-    {
-        ALCopenslPlayback_lock(self);
         aluHandleDisconnect(device, "Failed to get playback buffer: 0x%08x", result);
-        ALCopenslPlayback_unlock(self);
-        return 1;
-    }
 
-    ALCopenslPlayback_lock(self);
-    while(!ATOMIC_LOAD(&self->mKillNow, almemory_order_acquire) &&
+    while(SL_RESULT_SUCCESS == result &&
+          !ATOMIC_LOAD(&self->mKillNow, almemory_order_acquire) &&
           ATOMIC_LOAD(&device->Connected, almemory_order_acquire))
     {
-        size_t todo, len0, len1;
+        size_t todo;
 
         if(ll_ringbuffer_write_space(self->mRing) == 0)
         {
@@ -292,34 +292,33 @@ static int ALCopenslPlayback_mixerProc(void *arg)
         }
 
         ll_ringbuffer_get_write_vector(self->mRing, data);
-        todo = data[0].len+data[1].len;
-
-        len0 = minu(todo, data[0].len);
-        len1 = minu(todo-len0, data[1].len);
 
-        aluMixData(device, data[0].buf, len0*device->UpdateSize);
-        for(size_t i = 0;i < len0;i++)
-        {
-            result = VCALL(bufferQueue,Enqueue)(data[0].buf, device->UpdateSize*self->mFrameSize);
-            PRINTERR(result, "bufferQueue->Enqueue");
-            if(SL_RESULT_SUCCESS == result)
-                ll_ringbuffer_write_advance(self->mRing, 1);
+        aluMixData(device, data[0].buf, data[0].len*device->UpdateSize);
+        if(data[1].len > 0)
+            aluMixData(device, data[1].buf, data[1].len*device->UpdateSize);
 
-            data[0].buf += device->UpdateSize*self->mFrameSize;
-        }
+        todo = data[0].len+data[1].len;
+        ll_ringbuffer_write_advance(self->mRing, todo);
 
-        if(len1 > 0)
+        for(size_t i = 0;i < todo;i++)
         {
-            aluMixData(device, data[1].buf, len1*device->UpdateSize);
-            for(size_t i = 0;i < len1;i++)
+            if(!data[0].len)
             {
-                result = VCALL(bufferQueue,Enqueue)(data[1].buf, device->UpdateSize*self->mFrameSize);
-                PRINTERR(result, "bufferQueue->Enqueue");
-                if(SL_RESULT_SUCCESS == result)
-                    ll_ringbuffer_write_advance(self->mRing, 1);
+                data[0] = data[1];
+                data[1].buf = NULL;
+                data[1].len = 0;
+            }
 
-                data[1].buf += device->UpdateSize*self->mFrameSize;
+            result = VCALL(bufferQueue,Enqueue)(data[0].buf, device->UpdateSize*self->mFrameSize);
+            PRINTERR(result, "bufferQueue->Enqueue");
+            if(SL_RESULT_SUCCESS != result)
+            {
+                aluHandleDisconnect(device, "Failed to queue audio: 0x%08x", result);
+                break;
             }
+
+            data[0].len--;
+            data[0].buf += device->UpdateSize*self->mFrameSize;
         }
     }
     ALCopenslPlayback_unlock(self);
@@ -398,6 +397,9 @@ static ALCboolean ALCopenslPlayback_reset(ALCopenslPlayback *self)
         VCALL0(self->mBufferQueueObj,Destroy)();
     self->mBufferQueueObj = NULL;
 
+    ll_ringbuffer_free(self->mRing);
+    self->mRing = NULL;
+
     sampleRate = device->Frequency;
     if(!(device->Flags&DEVICE_FREQUENCY_REQUEST) && (env=Android_GetJNIEnv()) != NULL)
     {
@@ -546,6 +548,18 @@ static ALCboolean ALCopenslPlayback_reset(ALCopenslPlayback *self)
         result = VCALL(self->mBufferQueueObj,Realize)(SL_BOOLEAN_FALSE);
         PRINTERR(result, "bufferQueue->Realize");
     }
+    if(SL_RESULT_SUCCESS == result)
+    {
+        self->mRing = ll_ringbuffer_create(device->NumUpdates,
+            self->mFrameSize*device->UpdateSize, true
+        );
+        if(!self->mRing)
+        {
+            ERR("Out of memory allocating ring buffer %ux%u %u\n", device->UpdateSize,
+                device->NumUpdates, self->mFrameSize);
+            result = SL_RESULT_MEMORY_FAILURE;
+        }
+    }
 
     if(SL_RESULT_SUCCESS != result)
     {
@@ -561,13 +575,10 @@ static ALCboolean ALCopenslPlayback_reset(ALCopenslPlayback *self)
 
 static ALCboolean ALCopenslPlayback_start(ALCopenslPlayback *self)
 {
-    ALCdevice *device = STATIC_CAST(ALCbackend,self)->mDevice;
     SLAndroidSimpleBufferQueueItf bufferQueue;
     SLresult result;
 
-    ll_ringbuffer_free(self->mRing);
-    self->mRing = ll_ringbuffer_create(device->NumUpdates, self->mFrameSize*device->UpdateSize,
-                                       true);
+    ll_ringbuffer_reset(self->mRing);
 
     result = VCALL(self->mBufferQueueObj,GetInterface)(SL_IID_ANDROIDSIMPLEBUFFERQUEUE,
                                                        &bufferQueue);
@@ -634,9 +645,6 @@ static void ALCopenslPlayback_stop(ALCopenslPlayback *self)
         } while(SL_RESULT_SUCCESS == result && state.count > 0);
         PRINTERR(result, "bufferQueue->GetState");
     }
-
-    ll_ringbuffer_free(self->mRing);
-    self->mRing = NULL;
 }
 
 static ClockLatency ALCopenslPlayback_getClockLatency(ALCopenslPlayback *self)
@@ -713,9 +721,6 @@ static void ALCopenslCapture_Construct(ALCopenslCapture *self, ALCdevice *device
 
 static void ALCopenslCapture_Destruct(ALCopenslCapture *self)
 {
-    ll_ringbuffer_free(self->mRing);
-    self->mRing = NULL;
-
     if(self->mRecordObj != NULL)
         VCALL0(self->mRecordObj,Destroy)();
     self->mRecordObj = NULL;
@@ -725,6 +730,9 @@ static void ALCopenslCapture_Destruct(ALCopenslCapture *self)
     self->mEngineObj = NULL;
     self->mEngine = NULL;
 
+    ll_ringbuffer_free(self->mRing);
+    self->mRing = NULL;
+
     ALCbackend_Destruct(STATIC_CAST(ALCbackend, self));
 }
 
@@ -843,8 +851,9 @@ static ALCenum ALCopenslCapture_open(ALCopenslCapture *self, const ALCchar *name
 
     if(SL_RESULT_SUCCESS == result)
     {
-        self->mRing = ll_ringbuffer_create(device->NumUpdates, device->UpdateSize*self->mFrameSize,
-                                           false);
+        self->mRing = ll_ringbuffer_create(device->NumUpdates,
+            device->UpdateSize*self->mFrameSize, false
+        );
 
         result = VCALL(self->mRecordObj,GetInterface)(SL_IID_ANDROIDSIMPLEBUFFERQUEUE,
                                                       &bufferQueue);
@@ -972,14 +981,25 @@ static ALCenum ALCopenslCapture_captureSamples(ALCopenslCapture *self, ALCvoid *
 
         i += rem;
     }
+    if(!advance)
+        return ALC_NO_ERROR;
     ll_ringbuffer_read_advance(self->mRing, advance);
 
     result = VCALL(self->mRecordObj,GetInterface)(SL_IID_ANDROIDSIMPLEBUFFERQUEUE,
                                                   &bufferQueue);
     PRINTERR(result, "recordObj->GetInterface");
 
-    /* Enqueue any newly-writable chunks in the ring buffer. */
+    /* Enqueue any newly-writable chunks in the ring buffer. Limit the number
+     * of enqueued chunks to the number of fully read chunks.
+     */
     ll_ringbuffer_get_write_vector(self->mRing, data);
+    if(data[0].len > advance)
+    {
+        data[0].len = advance;
+        data[1].len = 0;
+    }
+    else if(data[1].len > advance-data[0].len)
+        data[1].len = advance-data[0].len;
     for(i = 0;i < data[0].len && SL_RESULT_SUCCESS == result;i++)
     {
         result = VCALL(bufferQueue,Enqueue)(data[0].buf + chunk_size*i, chunk_size);