[vlmc-devel] ClipWorkflow: Fix a potential deadlock

[Hugo Beauzée-Luyssen]

vlmc | branch: master | Hugo Beauzée-Luyssen <hugo at beauzee.fr> | Sun Mar 27 16:57:29 2016 +0200| [5fb5748cfeb93d1b684fab5c79687ced414f5af5] | committer: Hugo Beauzée-Luyssen

ClipWorkflow: Fix a potential deadlock

Checking for available buffers queue from the clipworkflow renderer is
dangerous, and can cause a deadlock if the workflow is stopped during
this operation.
It makes more sense to control the workflow from an outside thread

> https://code.videolan.org/videolan/vlmc/commit/5fb5748cfeb93d1b684fab5c79687ced414f5af5
---

 src/Workflow/AudioClipWorkflow.cpp | 1 -
 src/Workflow/ClipWorkflow.cpp      | 9 +++------
 src/Workflow/ClipWorkflow.h        | 1 -
 src/Workflow/VideoClipWorkflow.cpp | 1 -
 4 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/src/Workflow/AudioClipWorkflow.cpp b/src/Workflow/AudioClipWorkflow.cpp
index 6622560..98b21f7 100644
--- a/src/Workflow/AudioClipWorkflow.cpp
+++ b/src/Workflow/AudioClipWorkflow.cpp
@@ -168,7 +168,6 @@ AudioClipWorkflow::unlock( void* data, uint8_t *pcm_buffer, unsigned int channel
         else
             cw->m_currentPts = pts;
     }
-    cw->commonUnlock();
     cw->m_renderLock->unlock();
 }
 
diff --git a/src/Workflow/ClipWorkflow.cpp b/src/Workflow/ClipWorkflow.cpp
index 8fbb35c..d54713c 100644
--- a/src/Workflow/ClipWorkflow.cpp
+++ b/src/Workflow/ClipWorkflow.cpp
@@ -160,15 +160,12 @@ ClipWorkflow::postGetOutput()
     //If we're running out of computed buffers, refill our stack.
     if ( getNbComputedBuffers() < getMaxComputedBuffers() / 3 )
         m_renderer->setPause( false );
-}
-
-void
-ClipWorkflow::commonUnlock()
-{
     //Don't test using availableBuffer, as it may evolve if a buffer is required while
     //no one is available : we would spawn a new buffer, thus modifying the number of available buffers
-    if ( getNbComputedBuffers() >= getMaxComputedBuffers() )
+    else if ( getNbComputedBuffers() >= getMaxComputedBuffers() )
     {
+        // It's OK to check from here: if getOutput is not called, it means the clipworkflow is
+        // stopped or preloading, in which case, we don't care about the buffer queue growing uncontrolled
         m_renderer->setPause( true );
     }
 }
diff --git a/src/Workflow/ClipWorkflow.h b/src/Workflow/ClipWorkflow.h
index 619763e..ea920a7 100644
--- a/src/Workflow/ClipWorkflow.h
+++ b/src/Workflow/ClipWorkflow.h
@@ -176,7 +176,6 @@ class   ClipWorkflow : public EffectUser
 
     protected:
         void                    computePtsDiff( qint64 pts );
-        void                    commonUnlock();
         /**
          *  \warning    Must be called from a thread safe context.
          *              This thread safe context has to be set
diff --git a/src/Workflow/VideoClipWorkflow.cpp b/src/Workflow/VideoClipWorkflow.cpp
index 5e9faf6..f6484e5 100644
--- a/src/Workflow/VideoClipWorkflow.cpp
+++ b/src/Workflow/VideoClipWorkflow.cpp
@@ -162,7 +162,6 @@ VideoClipWorkflow::unlock( void *data, uint8_t *buffer, int width,
     cw->computePtsDiff( pts );
     Workflow::Frame     *frame = cw->m_computedBuffers.last();
     frame->ptsDiff = cw->m_currentPts - cw->m_previousPts;
-    cw->commonUnlock();
     cw->m_renderWaitCond->wakeAll();
     cw->m_renderLock->unlock();
 }