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.
Unified · Split
| @@ -27,6 +27,7 @@ · 26 unmodified lines | |||
| 27 | #include "libavutil/channel_layout.h" | 27 | #include "libavutil/channel_layout.h" |
| 28 | #include "libavutil/file.h" | 28 | #include "libavutil/file.h" |
| 29 | #include "libavutil/opt.h" | 29 | #include "libavutil/opt.h" |
| 30 | #include "libavutil/thread.h" | ||
| 30 | #include "avfilter.h" | 31 | #include "avfilter.h" |
| 31 | #include "audio.h" | 32 | #include "audio.h" |
| 32 | #include "formats.h" | 33 | #include "formats.h" |
| @@ -63,7 +64,9 @@ static const AVOption flite_options[] = { · 30 unmodified lines | |||
| 63 | 64 | ||
| 64 | AVFILTER_DEFINE_CLASS(flite); | 65 | AVFILTER_DEFINE_CLASS(flite); |
| 65 | 66 | ||
| 66 | static volatile int flite_inited = 0; | 67 | static AVMutex flite_mutex = AV_MUTEX_INITIALIZER; |
| 68 | |||
| 69 | static int flite_inited = 0; | ||
| 67 | 70 | ||
| 68 | /* declare functions for all the supported voices */ | 71 | /* declare functions for all the supported voices */ |
| 69 | #define DECLARE_REGISTER_VOICE_FN(name) \ | 72 | #define DECLARE_REGISTER_VOICE_FN(name) \ |
| @@ -111,14 +114,19 @@ static int select_voice(struct voice_entry **entry_ret, const char *voice_name, · 41 unmodified lines | |||
| 111 | for (i = 0; i < FF_ARRAY_ELEMS(voice_entries); i++) { | 114 | for (i = 0; i < FF_ARRAY_ELEMS(voice_entries); i++) { |
| 112 | struct voice_entry *entry = &voice_entries[i]; | 115 | struct voice_entry *entry = &voice_entries[i]; |
| 113 | if (!strcmp(entry->name, voice_name)) { | 116 | if (!strcmp(entry->name, voice_name)) { |
| 117 | cst_voice *voice; | ||
| 118 | pthread_mutex_lock(&flite_mutex); | ||
| 114 | if (!entry->voice) | 119 | if (!entry->voice) |
| 115 | entry->voice = entry->register_fn(NULL); | 120 | entry->voice = entry->register_fn(NULL); |
| 116 | if (!entry->voice) { | 121 | voice = entry->voice; |
| 122 | if (voice) | ||
| 123 | entry->usage_count++; | ||
| 124 | pthread_mutex_unlock(&flite_mutex); | ||
| 125 | if (!voice) { | ||
| 117 | av_log(log_ctx, AV_LOG_ERROR, | 126 | av_log(log_ctx, AV_LOG_ERROR, |
| 118 | "Could not register voice '%s'\n", voice_name); | 127 | "Could not register voice '%s'\n", voice_name); |
| 119 | return AVERROR_UNKNOWN; | 128 | return AVERROR_UNKNOWN; |
| 120 | } | 129 | } |
| 121 | entry->usage_count++; | ||
| 122 | *entry_ret = entry; | 130 | *entry_ret = entry; |
| 123 | return 0; | 131 | return 0; |
| 124 | } | 132 | } |
| @@ -141,12 +149,15 @@ static av_cold int init(AVFilterContext *ctx) · 16 unmodified lines | |||
| 141 | return AVERROR_EXIT; | 149 | return AVERROR_EXIT; |
| 142 | } | 150 | } |
| 143 | 151 | ||
| 152 | pthread_mutex_lock(&flite_mutex); | ||
| 144 | if (!flite_inited) { | 153 | if (!flite_inited) { |
| 145 | if (flite_init() < 0) { | 154 | if ((ret = flite_init()) >= 0) |
| 146 | av_log(ctx, AV_LOG_ERROR, "flite initialization failed\n"); | 155 | flite_inited = 1; |
| 147 | return AVERROR_UNKNOWN; | 156 | } |
| 148 | } | 157 | pthread_mutex_unlock(&flite_mutex); |
| 149 | flite_inited++; | 158 | if (ret < 0) { |
| 159 | av_log(ctx, AV_LOG_ERROR, "flite initialization failed\n"); | ||
| 160 | return AVERROR_EXTERNAL; | ||
| 150 | } | 161 | } |
| 151 | 162 | ||
| 152 | if ((ret = select_voice(&flite->voice_entry, flite->voice_str, ctx)) < 0) | 163 | if ((ret = select_voice(&flite->voice_entry, flite->voice_str, ctx)) < 0) |
| @@ -197,10 +208,12 @@ static av_cold void uninit(AVFilterContext *ctx) · 44 unmodified lines | |||
| 197 | FliteContext *flite = ctx->priv; | 208 | FliteContext *flite = ctx->priv; |
| 198 | 209 | ||
| 199 | if (flite->voice_entry) { | 210 | if (flite->voice_entry) { |
| 211 | pthread_mutex_lock(&flite_mutex); | ||
| 200 | if (!--flite->voice_entry->usage_count) { | 212 | if (!--flite->voice_entry->usage_count) { |
| 201 | flite->voice_entry->unregister_fn(flite->voice); | 213 | flite->voice_entry->unregister_fn(flite->voice); |
| 202 | flite->voice_entry->voice = NULL; | 214 | flite->voice_entry->voice = NULL; |
| 203 | } | 215 | } |
| 216 | pthread_mutex_unlock(&flite_mutex); | ||
| 204 | } | 217 | } |
| 205 | delete_wave(flite->wave); | 218 | delete_wave(flite->wave); |
| 206 | flite->wave = NULL; | 219 | flite->wave = NULL; |