| Message ID | 20201128232852.115729-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | 0639f5c294c70c55f4da75ce2ca5bf6c5a809248 |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/pthread_slice: Don't use static variable, fix race | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
Quoting Andreas Rheinhardt (2020-11-29 00:28:51) > ff_slice_thread_init() uses a static variable to hold a function > pointer, although the value of said pointer needn't be saved between > different runs of this function at all; > > The reason for this being so is probably that said pointer points to > a static function (if used); but storage class specifiers like "static" > are not part of the type of an object and so including it in the pointer > declaration is wrong (anyway, "static" means different things in both > contexts: for the function declaration it affects linkage, for the > variable storage duration). > > Using a static variable here can lead to races, e.g. when initializing > VP9 (for which said function pointer was added) and H.264 with slice > threading. The latter has the FF_CODEC_CAP_INIT_THREADSAFE flag set and > is therefore unaffected by the lock guarding initializations of > decoders. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- Looks very good.
diff --git a/libavcodec/pthread_slice.c b/libavcodec/pthread_slice.c index 77cfe3c9f6..80c4579fc0 100644 --- a/libavcodec/pthread_slice.c +++ b/libavcodec/pthread_slice.c @@ -130,7 +130,7 @@ int ff_slice_thread_init(AVCodecContext *avctx) { SliceThreadContext *c; int thread_count = avctx->thread_count; - static void (*mainfunc)(void *); + void (*mainfunc)(void *); // We cannot do this in the encoder init as the threads are created before if (av_codec_is_encoder(avctx->codec) &&
ff_slice_thread_init() uses a static variable to hold a function pointer, although the value of said pointer needn't be saved between different runs of this function at all; The reason for this being so is probably that said pointer points to a static function (if used); but storage class specifiers like "static" are not part of the type of an object and so including it in the pointer declaration is wrong (anyway, "static" means different things in both contexts: for the function declaration it affects linkage, for the variable storage duration). Using a static variable here can lead to races, e.g. when initializing VP9 (for which said function pointer was added) and H.264 with slice threading. The latter has the FF_CODEC_CAP_INIT_THREADSAFE flag set and is therefore unaffected by the lock guarding initializations of decoders. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavcodec/pthread_slice.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)