Thomas Guillem: > > > On Thu, Dec 2, 2021, at 14:38, Andreas Rheinhardt wrote: >> Thomas Guillem: >>> Reproduced when using the VAAPI va module on VLC 4.0. No leaks when >>> setting thread count to 1. >>> --- >>> libavcodec/pthread_frame.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c >>> index 73b1b7d7d9..4c578aa44a 100644 >>> --- a/libavcodec/pthread_frame.c >>> +++ b/libavcodec/pthread_frame.c >>> @@ -747,6 +747,7 @@ void ff_frame_thread_free(AVCodecContext *avctx, int >>> thread_count) >>> av_buffer_unref(&ctx->internal->pool); >>> av_freep(&ctx->internal); >>> av_buffer_unref(&ctx->hw_frames_ctx); >>> + av_buffer_unref(&ctx->hw_device_ctx); >>> } >>> >>> av_frame_free(&p->frame); >>> >> >> The AVCodecContext that is freed here is not a full AVCodecContext: It >> never received a reference to hw_device_ctx of its own. Unreferencing >> this here will therefore mess up the refcount and lead to use-after-frees. >> (What is the reference count of hw_device_ctx at this point? Libavcodec >> should only hold one reference at that point, namely the one in the main >> (user-facing) AVCodecContext; this reference will be unreferenced when >> avcodec_close()/avcodec_free_context() is called for the main context.) > > Hello Andreas, thanks for the review. > > The problem is that hw_device_ctx is NULL when avcodec_close() is called in > that particular usecase. My first guess is that the get_format callback can > be called from any avctx (and not necessarily the main one that will be > closed by avcodec_close(). > > I'm new to this code, but I thought that only one FrameThreadContext could > meet the if (ctx->internal) condition, so there should be only one reference > to the hw_device_ctx. >
1. hw_device_ctx is in the public AVCodecContext, not in the private AVCodecInternal. 2. ctx->internal is intended to be allocated for all the internal AVCodecContexts; the check is only there to ensure that we don't crash if an allocation error happens. 3. libavcodec only ever modifies avctx->hw_device_ctx at one place: in avcodec_close() where is unreferences the reference (if any). So if it was set in avcodec_open2() and NULL in avcodec_close() (i.e. immediately before avcodec_close()), it has been modified somehow and you need to find out who did it. 4. Some parts of the code use hw_device_ctx to derive references from it. Maybe one of these references did not get freed? (5. What do Valgrind/ASAN say? When was the reference that leaked created?) - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".