diff mbox series

[FFmpeg-devel,7/8] avfilter/asrc_flite: Fix races upon (un)registering voices

Message ID AM7PR03MB66601A6A4A02AF437870A11A8FB19@AM7PR03MB6660.eurprd03.prod.outlook.com
State Accepted
Commit aac8fa2fbfe235f4a6c3ea733295dd59a7bae8f7
Headers show
Series [FFmpeg-devel,1/8] avfilter/vf_w3fdif: Fix segfault on allocation error | expand


Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Andreas Rheinhardt Oct. 7, 2021, 9:31 a.m. UTC
The voice registration system in libflite is broken: It is not
thread-safe and also not based on internal counters; instead
any call to unregister a voice frees said voice even if there are still
many other users of said voice who have also registered said voice.
While there is no way to guard against another library unregistering
voices behind our back, we can at least be correct in the absence of
other users of libflite. The current code already tried this by using
a reference count of our own for each voice; but the implementation
of this is not thread-safe at all.

Fix this by using a mutex to guard all of libavfilter's libflite
registration and unregistration calls, thereby being thread-safe
in the absence of other libflite users.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
I wonder whether we should unregister voices at all; after all,
there might be a non-libavfilter user of these voices left who
can't guard against us unregistering this voice.
If we did so, the reference counters could be removed.
If we also stopped worrying about calling the registration/init functions
multiple times (as long as we don't do it concurrently), we could
remove storing the voices in voice_entry and make voice_entries const:
Every user would just have to register the voice itself.

 libavfilter/asrc_flite.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)
diff mbox series


diff --git a/libavfilter/asrc_flite.c b/libavfilter/asrc_flite.c
index bd2ae774de..6335a81f0e 100644
--- a/libavfilter/asrc_flite.c
+++ b/libavfilter/asrc_flite.c
@@ -27,6 +27,7 @@ 
 #include "libavutil/channel_layout.h"
 #include "libavutil/file.h"
 #include "libavutil/opt.h"
+#include "libavutil/thread.h"
 #include "avfilter.h"
 #include "audio.h"
 #include "formats.h"
@@ -63,7 +64,9 @@  static const AVOption flite_options[] = {
-static volatile int flite_inited = 0;
+static AVMutex flite_mutex = AV_MUTEX_INITIALIZER;
+static int flite_inited = 0;
 /* declare functions for all the supported voices */
@@ -111,14 +114,19 @@  static int select_voice(struct voice_entry **entry_ret, const char *voice_name,
     for (i = 0; i < FF_ARRAY_ELEMS(voice_entries); i++) {
         struct voice_entry *entry = &voice_entries[i];
         if (!strcmp(entry->name, voice_name)) {
+            cst_voice *voice;
+            pthread_mutex_lock(&flite_mutex);
             if (!entry->voice)
                 entry->voice = entry->register_fn(NULL);
-            if (!entry->voice) {
+            voice = entry->voice;
+            if (voice)
+                entry->usage_count++;
+            pthread_mutex_unlock(&flite_mutex);
+            if (!voice) {
                 av_log(log_ctx, AV_LOG_ERROR,
                        "Could not register voice '%s'\n", voice_name);
                 return AVERROR_UNKNOWN;
-            entry->usage_count++;
             *entry_ret = entry;
             return 0;
@@ -141,12 +149,15 @@  static av_cold int init(AVFilterContext *ctx)
         return AVERROR_EXIT;
+    pthread_mutex_lock(&flite_mutex);
     if (!flite_inited) {
-        if (flite_init() < 0) {
-            av_log(ctx, AV_LOG_ERROR, "flite initialization failed\n");
-            return AVERROR_UNKNOWN;
-        }
-        flite_inited++;
+        if ((ret = flite_init()) >= 0)
+            flite_inited = 1;
+    }
+    pthread_mutex_unlock(&flite_mutex);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "flite initialization failed\n");
+        return AVERROR_EXTERNAL;
     if ((ret = select_voice(&flite->voice_entry, flite->voice_str, ctx)) < 0)
@@ -197,10 +208,12 @@  static av_cold void uninit(AVFilterContext *ctx)
     FliteContext *flite = ctx->priv;
     if (flite->voice_entry) {
+        pthread_mutex_lock(&flite_mutex);
         if (!--flite->voice_entry->usage_count) {
             flite->voice_entry->voice = NULL;
+        pthread_mutex_unlock(&flite_mutex);
     flite->wave = NULL;