[FFmpeg-devel] [PATCH] avutil/eval: make av_expr_eval more thread safe

[Marton Balint]

Unfortunately the ld() and st() operations store the variables in the AVExpr
context and not in the parser in order to keep the stored values between
evaluations (which is a rarely(?) used undocumented(?) feature of AVExpr).

This causes data race issues when the same expression is evaluated from
multiple threads. Let's use the Parser as variable store during evaluation and
only sync the AVExpr context variables with the Parser before and after
evaluation. This keeps the feature working for single threaded cases and fixes
using ld() and st() usage for the "normal" use case when the ld()-s only
reference st()-d values from the same evaluation.

Maybe we should just remove this feature instead?

Fixes ticket #7528.

Signed-off-by: Marton Balint <cus at passwd.hu>
---
 libavutil/eval.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libavutil/eval.c b/libavutil/eval.c
index 62d2ae938b..e726c94195 100644
--- a/libavutil/eval.c
+++ b/libavutil/eval.c
@@ -54,7 +54,7 @@ typedef struct Parser {
     int log_offset;
     void *log_ctx;
 #define VARS 10
-    double *var;
+    double var[VARS];
 } Parser;
 
 static const AVClass eval_class = {
@@ -754,11 +754,14 @@ int av_expr_count_vars(AVExpr *e, unsigned *counter, int size)
 double av_expr_eval(AVExpr *e, const double *const_values, void *opaque)
 {
     Parser p = { 0 };
-    p.var= e->var;
+    double ret;
 
+    memcpy(p.var, e->var, sizeof(p.var));
     p.const_values = const_values;
     p.opaque     = opaque;
-    return eval_expr(&p, e);
+    ret = eval_expr(&p, e);
+    memcpy(e->var, p.var, sizeof(p.var));
+    return ret;
 }
 
 int av_expr_parse_and_eval(double *d, const char *s,
-- 
2.16.4