From patchwork Sun Dec 15 01:16:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marton Balint X-Patchwork-Id: 16799 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 818D9443749 for ; Sun, 15 Dec 2019 03:17:10 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 65860687F5E; Sun, 15 Dec 2019 03:17:10 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from iq.passwd.hu (iq.passwd.hu [217.27.212.140]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0C830680060 for ; Sun, 15 Dec 2019 03:17:03 +0200 (EET) Received: from localhost (localhost [127.0.0.1]) by iq.passwd.hu (Postfix) with ESMTP id DBE54E3B35; Sun, 15 Dec 2019 02:17:02 +0100 (CET) X-Virus-Scanned: amavisd-new at passwd.hu Received: from iq.passwd.hu ([127.0.0.1]) by localhost (iq.passwd.hu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id U3wiur3AuJkH; Sun, 15 Dec 2019 02:17:01 +0100 (CET) Received: from bluegene.passwd.hu (localhost [127.0.0.1]) by iq.passwd.hu (Postfix) with ESMTP id 66690E39D8; Sun, 15 Dec 2019 02:17:01 +0100 (CET) From: Marton Balint To: ffmpeg-devel@ffmpeg.org Date: Sun, 15 Dec 2019 02:16:49 +0100 Message-Id: <20191215011649.12202-1-cus@passwd.hu> X-Mailer: git-send-email 2.16.4 Subject: [FFmpeg-devel] [PATCH] avutil/eval: make av_expr_eval more thread safe X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Marton Balint MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 --- 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,