diff mbox

[FFmpeg-devel] lavfi: fix race when func rets holder is NULL

Message ID 20170328155929.13541-1-u@pkh.me
State Accepted
Commit 473f0f75a16b4d37bdaa943f75e4ae249212c1ba
Headers show

Commit Message

Clément Bœsch March 28, 2017, 3:59 p.m. UTC 
From: Clément Bœsch <cboesch@gopro.com>

If ret is NULL, a dummy common holder is created to hold *all* the
parallel function returns, which gets written concurrently. This commit
simplify the whole logic by simply not writing to that holder when not
set.
---
 libavfilter/pthread.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

Comments

Michael Niedermayer March 28, 2017, 9:39 p.m. UTC | #1 
On Tue, Mar 28, 2017 at 05:59:29PM +0200, Clément Bœsch wrote:
> From: Clément Bœsch <cboesch@gopro.com>
> 
> If ret is NULL, a dummy common holder is created to hold *all* the
> parallel function returns, which gets written concurrently. This commit
> simplify the whole logic by simply not writing to that holder when not
> set.
> ---
>  libavfilter/pthread.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)

should be ok

thx

[...]
Clément Bœsch March 28, 2017, 9:48 p.m. UTC | #2 
On Tue, Mar 28, 2017 at 11:39:49PM +0200, Michael Niedermayer wrote:
> On Tue, Mar 28, 2017 at 05:59:29PM +0200, Clément Bœsch wrote:
> > From: Clément Bœsch <cboesch@gopro.com>
> > 
> > If ret is NULL, a dummy common holder is created to hold *all* the
> > parallel function returns, which gets written concurrently. This commit
> > simplify the whole logic by simply not writing to that holder when not
> > set.
> > ---
> >  libavfilter/pthread.c | 17 ++++++-----------
> >  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> should be ok
> 
> thx
> 

applied, thanks
diff mbox

Patch

diff --git a/libavfilter/pthread.c b/libavfilter/pthread.c
index ccb915eae5..c7a00210d6 100644
--- a/libavfilter/pthread.c
+++ b/libavfilter/pthread.c
@@ -43,7 +43,6 @@  typedef struct ThreadContext {
     AVFilterContext *ctx;
     void *arg;
     int   *rets;
-    int nb_rets;
     int nb_jobs;
 
     pthread_cond_t last_job_cond;
@@ -60,10 +59,11 @@  static void* attribute_align_arg worker(void *v)
     int our_job      = c->nb_jobs;
     int nb_threads   = c->nb_threads;
     unsigned int last_execute = 0;
-    int self_id;
+    int ret, self_id;
 
     pthread_mutex_lock(&c->current_job_lock);
     self_id = c->current_job++;
+
     for (;;) {
         while (our_job >= c->nb_jobs) {
             if (c->current_job == nb_threads + c->nb_jobs)
@@ -81,7 +81,9 @@  static void* attribute_align_arg worker(void *v)
         }
         pthread_mutex_unlock(&c->current_job_lock);
 
-        c->rets[our_job % c->nb_rets] = c->func(c->ctx, c->arg, our_job, c->nb_jobs);
+        ret = c->func(c->ctx, c->arg, our_job, c->nb_jobs);
+        if (c->rets)
+            c->rets[our_job % c->nb_jobs] = ret;
 
         pthread_mutex_lock(&c->current_job_lock);
         our_job = c->current_job++;
@@ -117,7 +119,6 @@  static int thread_execute(AVFilterContext *ctx, avfilter_action_func *func,
                           void *arg, int *ret, int nb_jobs)
 {
     ThreadContext *c = ctx->graph->internal->thread;
-    int dummy_ret;
 
     if (nb_jobs <= 0)
         return 0;
@@ -129,13 +130,7 @@  static int thread_execute(AVFilterContext *ctx, avfilter_action_func *func,
     c->ctx         = ctx;
     c->arg         = arg;
     c->func        = func;
-    if (ret) {
-        c->rets    = ret;
-        c->nb_rets = nb_jobs;
-    } else {
-        c->rets    = &dummy_ret;
-        c->nb_rets = 1;
-    }
+    c->rets        = ret;
     c->current_execute++;
 
     pthread_cond_broadcast(&c->current_job_cond);