| Message ID | AM7PR03MB66603C1F7536E128C4E977FE8FAB9@AM7PR03MB6660.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Commit | 5a261cfa32d2833d20d87414bef267f3765abc4d |
| Headers | show |
| Series | [FFmpeg-devel,01/20] libpostproc/postprocess_template: Don't reimplement FFSWAP | 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 |
Andreas Rheinhardt: > Up until now setting the input and output devices lists is guarded > by a mutex. This prevents data races emanating from multiple concurrent > calls to avpriv_register_devices() (triggered by multiple concurrent > calls to avdevice_register_all()). Yet reading the lists pointers was > done without any lock and with nonatomic variables. This means that > there are data races in case of concurrent calls to > av_(de)muxer_iterate() and avdevice_register_all() (but only if the > iteration in av_(de)muxer_iterate exhausts the non-device (de)muxers). > > This commit fixes this by putting said pointers into atomic objects. > Due to the unavailability of _Atomic the object is an atomic_uintptr, > leading to ugly casts. Switching to atomics also allowed to remove > the mutex currently used in avpriv_register_devices(). > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavformat/allformats.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/libavformat/allformats.c b/libavformat/allformats.c > index 5471f7c16f..cadaf057fc 100644 > --- a/libavformat/allformats.c > +++ b/libavformat/allformats.c > @@ -19,7 +19,7 @@ > * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > */ > > -#include "libavutil/thread.h" > +#include <stdatomic.h> > #include "libavformat/internal.h" > #include "avformat.h" > > @@ -534,18 +534,20 @@ extern const AVInputFormat ff_vapoursynth_demuxer; > #include "libavformat/muxer_list.c" > #include "libavformat/demuxer_list.c" > > -static const AVInputFormat * const *indev_list = NULL; > -static const AVOutputFormat * const *outdev_list = NULL; > +static atomic_uintptr_t indev_list_intptr = ATOMIC_VAR_INIT(0); > +static atomic_uintptr_t outdev_list_intptr = ATOMIC_VAR_INIT(0); > > const AVOutputFormat *av_muxer_iterate(void **opaque) > { > static const uintptr_t size = sizeof(muxer_list)/sizeof(muxer_list[0]) - 1; > uintptr_t i = (uintptr_t)*opaque; > const AVOutputFormat *f = NULL; > + uintptr_t tmp; > > if (i < size) { > f = muxer_list[i]; > - } else if (outdev_list) { > + } else if (tmp = atomic_load_explicit(&outdev_list_intptr, memory_order_relaxed)) { > + const AVOutputFormat *const *outdev_list = (const AVOutputFormat *const *)tmp; > f = outdev_list[i - size]; > } > > @@ -559,10 +561,12 @@ const AVInputFormat *av_demuxer_iterate(void **opaque) > static const uintptr_t size = sizeof(demuxer_list)/sizeof(demuxer_list[0]) - 1; > uintptr_t i = (uintptr_t)*opaque; > const AVInputFormat *f = NULL; > + uintptr_t tmp; > > if (i < size) { > f = demuxer_list[i]; > - } else if (indev_list) { > + } else if (tmp = atomic_load_explicit(&indev_list_intptr, memory_order_relaxed)) { > + const AVInputFormat *const *indev_list = (const AVInputFormat *const *)tmp; > f = indev_list[i - size]; > } > > @@ -571,12 +575,8 @@ const AVInputFormat *av_demuxer_iterate(void **opaque) > return f; > } > > -static AVMutex avpriv_register_devices_mutex = AV_MUTEX_INITIALIZER; > - > void avpriv_register_devices(const AVOutputFormat * const o[], const AVInputFormat * const i[]) > { > - ff_mutex_lock(&avpriv_register_devices_mutex); > - outdev_list = o; > - indev_list = i; > - ff_mutex_unlock(&avpriv_register_devices_mutex); > + atomic_store_explicit(&outdev_list_intptr, (uintptr_t)o, memory_order_relaxed); > + atomic_store_explicit(&indev_list_intptr, (uintptr_t)i, memory_order_relaxed); > } > Will apply this patch tomorrow unless there are objections. - Andreas
diff --git a/libavformat/allformats.c b/libavformat/allformats.c index 5471f7c16f..cadaf057fc 100644 --- a/libavformat/allformats.c +++ b/libavformat/allformats.c @@ -19,7 +19,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ -#include "libavutil/thread.h" +#include <stdatomic.h> #include "libavformat/internal.h" #include "avformat.h" @@ -534,18 +534,20 @@ extern const AVInputFormat ff_vapoursynth_demuxer; #include "libavformat/muxer_list.c" #include "libavformat/demuxer_list.c" -static const AVInputFormat * const *indev_list = NULL; -static const AVOutputFormat * const *outdev_list = NULL; +static atomic_uintptr_t indev_list_intptr = ATOMIC_VAR_INIT(0); +static atomic_uintptr_t outdev_list_intptr = ATOMIC_VAR_INIT(0); const AVOutputFormat *av_muxer_iterate(void **opaque) { static const uintptr_t size = sizeof(muxer_list)/sizeof(muxer_list[0]) - 1; uintptr_t i = (uintptr_t)*opaque; const AVOutputFormat *f = NULL; + uintptr_t tmp; if (i < size) { f = muxer_list[i]; - } else if (outdev_list) { + } else if (tmp = atomic_load_explicit(&outdev_list_intptr, memory_order_relaxed)) { + const AVOutputFormat *const *outdev_list = (const AVOutputFormat *const *)tmp; f = outdev_list[i - size]; } @@ -559,10 +561,12 @@ const AVInputFormat *av_demuxer_iterate(void **opaque) static const uintptr_t size = sizeof(demuxer_list)/sizeof(demuxer_list[0]) - 1; uintptr_t i = (uintptr_t)*opaque; const AVInputFormat *f = NULL; + uintptr_t tmp; if (i < size) { f = demuxer_list[i]; - } else if (indev_list) { + } else if (tmp = atomic_load_explicit(&indev_list_intptr, memory_order_relaxed)) { + const AVInputFormat *const *indev_list = (const AVInputFormat *const *)tmp; f = indev_list[i - size]; } @@ -571,12 +575,8 @@ const AVInputFormat *av_demuxer_iterate(void **opaque) return f; } -static AVMutex avpriv_register_devices_mutex = AV_MUTEX_INITIALIZER; - void avpriv_register_devices(const AVOutputFormat * const o[], const AVInputFormat * const i[]) { - ff_mutex_lock(&avpriv_register_devices_mutex); - outdev_list = o; - indev_list = i; - ff_mutex_unlock(&avpriv_register_devices_mutex); + atomic_store_explicit(&outdev_list_intptr, (uintptr_t)o, memory_order_relaxed); + atomic_store_explicit(&indev_list_intptr, (uintptr_t)i, memory_order_relaxed); }
Up until now setting the input and output devices lists is guarded by a mutex. This prevents data races emanating from multiple concurrent calls to avpriv_register_devices() (triggered by multiple concurrent calls to avdevice_register_all()). Yet reading the lists pointers was done without any lock and with nonatomic variables. This means that there are data races in case of concurrent calls to av_(de)muxer_iterate() and avdevice_register_all() (but only if the iteration in av_(de)muxer_iterate exhausts the non-device (de)muxers). This commit fixes this by putting said pointers into atomic objects. Due to the unavailability of _Atomic the object is an atomic_uintptr, leading to ugly casts. Switching to atomics also allowed to remove the mutex currently used in avpriv_register_devices(). Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/allformats.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)