diff mbox series

[FFmpeg-devel,2/2] avcodec/videotoolboxenc: fix use after destroy

Message ID tencent_8F402A9AAF9F44AB834E4470482DAE9A0308@qq.com
State Accepted
Commit d80d91d2135e416a0e50ac7127d95ac995f805f6
Headers show
Series [FFmpeg-devel,1/2] avcodec/videotoolboxenc: fix align issue
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

zhilizhao(赵志立) Aug. 26, 2020, 9:38 p.m. UTC
From: Zhao Zhili <zhilizhao@tencent.com>

The lock is used in clear_frame_queue().
---
 libavcodec/videotoolboxenc.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Steven Liu Aug. 28, 2020, 12:55 a.m. UTC | #1
<quinkblack@foxmail.com> 于2020年8月27日周四 下午5:39写道:
>
> From: Zhao Zhili <zhilizhao@tencent.com>
>
> The lock is used in clear_frame_queue().
> ---
>  libavcodec/videotoolboxenc.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> index 988782f10d..758db9641f 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -2496,14 +2496,17 @@ static av_cold int vtenc_close(AVCodecContext *avctx)
>  {
>      VTEncContext *vtctx = avctx->priv_data;
>
> -    pthread_cond_destroy(&vtctx->cv_sample_sent);
> -    pthread_mutex_destroy(&vtctx->lock);
> -
> -    if(!vtctx->session) return 0;
> +    if(!vtctx->session) {
> +        pthread_cond_destroy(&vtctx->cv_sample_sent);
> +        pthread_mutex_destroy(&vtctx->lock);
> +        return 0;
> +    }
>
>      VTCompressionSessionCompleteFrames(vtctx->session,
>                                         kCMTimeIndefinite);
>      clear_frame_queue(vtctx);
> +    pthread_cond_destroy(&vtctx->cv_sample_sent);
> +    pthread_mutex_destroy(&vtctx->lock);
>      CFRelease(vtctx->session);
>      vtctx->session = NULL;
>
> --
> 2.28.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".


This patch look better than
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200827025327.28334-1-lq@chinaffmpeg.org/


Thanks
Steven
zhilizhao(赵志立) Sept. 10, 2020, 12:56 p.m. UTC | #2
> On Aug 28, 2020, at 8:55 AM, Steven Liu <lingjiujianke@gmail.com> wrote:
> 
> <quinkblack@foxmail.com> 于2020年8月27日周四 下午5:39写道:
>> 
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> The lock is used in clear_frame_queue().
>> ---
>> libavcodec/videotoolboxenc.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>> 
>> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>> index 988782f10d..758db9641f 100644
>> --- a/libavcodec/videotoolboxenc.c
>> +++ b/libavcodec/videotoolboxenc.c
>> @@ -2496,14 +2496,17 @@ static av_cold int vtenc_close(AVCodecContext *avctx)
>> {
>>     VTEncContext *vtctx = avctx->priv_data;
>> 
>> -    pthread_cond_destroy(&vtctx->cv_sample_sent);
>> -    pthread_mutex_destroy(&vtctx->lock);
>> -
>> -    if(!vtctx->session) return 0;
>> +    if(!vtctx->session) {
>> +        pthread_cond_destroy(&vtctx->cv_sample_sent);
>> +        pthread_mutex_destroy(&vtctx->lock);
>> +        return 0;
>> +    }
>> 
>>     VTCompressionSessionCompleteFrames(vtctx->session,
>>                                        kCMTimeIndefinite);
>>     clear_frame_queue(vtctx);
>> +    pthread_cond_destroy(&vtctx->cv_sample_sent);
>> +    pthread_mutex_destroy(&vtctx->lock);
>>     CFRelease(vtctx->session);
>>     vtctx->session = NULL;
>> 
>> --
>> 2.28.0
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> 
> This patch look better than
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200827025327.28334-1-lq@chinaffmpeg.org/
> 

Ping for the patch set.

> 
> Thanks
> Steven
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Richard Kern Sept. 10, 2020, 1:59 p.m. UTC | #3
> On Sep 10, 2020, at 8:57 AM, zhilizhao <quinkblack@foxmail.com> wrote:
> 
> 
> 
>> On Aug 28, 2020, at 8:55 AM, Steven Liu <lingjiujianke@gmail.com> wrote:
>> 
>> <quinkblack@foxmail.com> 于2020年8月27日周四 下午5:39写道:
>>> 
>>> From: Zhao Zhili <zhilizhao@tencent.com>
>>> 
>>> The lock is used in clear_frame_queue().
>>> ---
>>> libavcodec/videotoolboxenc.c | 11 +++++++----
>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>>> index 988782f10d..758db9641f 100644
>>> --- a/libavcodec/videotoolboxenc.c
>>> +++ b/libavcodec/videotoolboxenc.c
>>> @@ -2496,14 +2496,17 @@ static av_cold int vtenc_close(AVCodecContext *avctx)
>>> {
>>>    VTEncContext *vtctx = avctx->priv_data;
>>> 
>>> -    pthread_cond_destroy(&vtctx->cv_sample_sent);
>>> -    pthread_mutex_destroy(&vtctx->lock);
>>> -
>>> -    if(!vtctx->session) return 0;
>>> +    if(!vtctx->session) {
>>> +        pthread_cond_destroy(&vtctx->cv_sample_sent);
>>> +        pthread_mutex_destroy(&vtctx->lock);
>>> +        return 0;
>>> +    }
>>> 
>>>    VTCompressionSessionCompleteFrames(vtctx->session,
>>>                                       kCMTimeIndefinite);
>>>    clear_frame_queue(vtctx);
>>> +    pthread_cond_destroy(&vtctx->cv_sample_sent);
>>> +    pthread_mutex_destroy(&vtctx->lock);
>>>    CFRelease(vtctx->session);
>>>    vtctx->session = NULL;
>>> 
>>> --
>>> 2.28.0
>>> 
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> 
>> 
>> This patch look better than
>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200827025327.28334-1-lq@chinaffmpeg.org/
>> 
> 
> Ping for the patch set.
Can you provide steps to reproduce the deadlock issue? I can get it pushed this weekend if so. 
> 
>> 
>> Thanks
>> Steven
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
zhilizhao(赵志立) Sept. 10, 2020, 3:13 p.m. UTC | #4
> 在 2020年9月10日,下午10:21,Richard Kern <kernrj@gmail.com> 写道:
> 
> 
> 
>> On Sep 10, 2020, at 8:57 AM, zhilizhao <quinkblack@foxmail.com> wrote:
>> 
>> 
>> 
>>>> On Aug 28, 2020, at 8:55 AM, Steven Liu <lingjiujianke@gmail.com> wrote:
>>> 
>>> <quinkblack@foxmail.com> 于2020年8月27日周四 下午5:39写道:
>>>> 
>>>> From: Zhao Zhili <zhilizhao@tencent.com>
>>>> 
>>>> The lock is used in clear_frame_queue().
>>>> ---
>>>> libavcodec/videotoolboxenc.c | 11 +++++++----
>>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>> 
>>>> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>>>> index 988782f10d..758db9641f 100644
>>>> --- a/libavcodec/videotoolboxenc.c
>>>> +++ b/libavcodec/videotoolboxenc.c
>>>> @@ -2496,14 +2496,17 @@ static av_cold int vtenc_close(AVCodecContext *avctx)
>>>> {
>>>>   VTEncContext *vtctx = avctx->priv_data;
>>>> 
>>>> -    pthread_cond_destroy(&vtctx->cv_sample_sent);
>>>> -    pthread_mutex_destroy(&vtctx->lock);
>>>> -
>>>> -    if(!vtctx->session) return 0;
>>>> +    if(!vtctx->session) {
>>>> +        pthread_cond_destroy(&vtctx->cv_sample_sent);
>>>> +        pthread_mutex_destroy(&vtctx->lock);
>>>> +        return 0;
>>>> +    }
>>>> 
>>>>   VTCompressionSessionCompleteFrames(vtctx->session,
>>>>                                      kCMTimeIndefinite);
>>>>   clear_frame_queue(vtctx);
>>>> +    pthread_cond_destroy(&vtctx->cv_sample_sent);
>>>> +    pthread_mutex_destroy(&vtctx->lock);
>>>>   CFRelease(vtctx->session);
>>>>   vtctx->session = NULL;
>>>> 
>>>> --
>>>> 2.28.0
>>>> 
>>>> _______________________________________________
>>>> ffmpeg-devel mailing list
>>>> ffmpeg-devel@ffmpeg.org
>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>> 
>>>> To unsubscribe, visit link above, or email
>>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>> 
>>> 
>>> This patch look better than
>>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200827025327.28334-1-lq@chinaffmpeg.org/
>>> 
>> 
>> Ping for the patch set.
> Can you provide steps to reproduce the deadlock issue? I can get it pushed this weekend if so. 

I only try to fix a use after destroy issue as the commit message says.
It may solved another issue unintentional. Liu, could you help by giving
more information about the deadlock issue, please.

>> 
>>> 
>>> Thanks
>>> Steven
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Richard Kern Sept. 17, 2020, 11:40 a.m. UTC | #5
> On Sep 10, 2020, at 11:13 AM, Zhao Zhili <quinkblack@foxmail.com> wrote:
> 
> 
>> 在 2020年9月10日,下午10:21,Richard Kern <kernrj@gmail.com> 写道:
>> 
>> 
>> 
>>>> On Sep 10, 2020, at 8:57 AM, zhilizhao <quinkblack@foxmail.com> wrote:
>>> 
>>> 
>>> 
>>>>> On Aug 28, 2020, at 8:55 AM, Steven Liu <lingjiujianke@gmail.com> wrote:
>>>> 
>>>> <quinkblack@foxmail.com> 于2020年8月27日周四 下午5:39写道:
>>>>> 
>>>>> From: Zhao Zhili <zhilizhao@tencent.com>
>>>>> 
>>>>> The lock is used in clear_frame_queue().
>>>>> ---
>>>>> libavcodec/videotoolboxenc.c | 11 +++++++----
>>>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>>> 
>>>>> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>>>>> index 988782f10d..758db9641f 100644
>>>>> --- a/libavcodec/videotoolboxenc.c
>>>>> +++ b/libavcodec/videotoolboxenc.c
>>>>> @@ -2496,14 +2496,17 @@ static av_cold int vtenc_close(AVCodecContext *avctx)
>>>>> {
>>>>>  VTEncContext *vtctx = avctx->priv_data;
>>>>> 
>>>>> -    pthread_cond_destroy(&vtctx->cv_sample_sent);
>>>>> -    pthread_mutex_destroy(&vtctx->lock);
>>>>> -
>>>>> -    if(!vtctx->session) return 0;
>>>>> +    if(!vtctx->session) {
>>>>> +        pthread_cond_destroy(&vtctx->cv_sample_sent);
>>>>> +        pthread_mutex_destroy(&vtctx->lock);
>>>>> +        return 0;
>>>>> +    }
>>>>> 
>>>>>  VTCompressionSessionCompleteFrames(vtctx->session,
>>>>>                                     kCMTimeIndefinite);
>>>>>  clear_frame_queue(vtctx);
>>>>> +    pthread_cond_destroy(&vtctx->cv_sample_sent);
>>>>> +    pthread_mutex_destroy(&vtctx->lock);
>>>>>  CFRelease(vtctx->session);
>>>>>  vtctx->session = NULL;
>>>>> 
>>>>> --
>>>>> 2.28.0
>>>>> 
>>>>> _______________________________________________
>>>>> ffmpeg-devel mailing list
>>>>> ffmpeg-devel@ffmpeg.org
>>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>> 
>>>>> To unsubscribe, visit link above, or email
>>>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>>> 
>>>> 
>>>> This patch look better than
>>>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200827025327.28334-1-lq@chinaffmpeg.org/
>>>> 
>>> 
>>> Ping for the patch set.
>> Can you provide steps to reproduce the deadlock issue? I can get it pushed this weekend if so. 
> 
> I only try to fix a use after destroy issue as the commit message says.
> It may solved another issue unintentional. Liu, could you help by giving
> more information about the deadlock issue, please.
I’ll figure out a way to reproduce this in the next few days and get these patches pushed. 

> 
>>> 
>>>> 
>>>> Thanks
>>>> Steven
>>>> _______________________________________________
>>>> ffmpeg-devel mailing list
>>>> ffmpeg-devel@ffmpeg.org
>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>> 
>>>> To unsubscribe, visit link above, or email
>>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>> 
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
index 988782f10d..758db9641f 100644
--- a/libavcodec/videotoolboxenc.c
+++ b/libavcodec/videotoolboxenc.c
@@ -2496,14 +2496,17 @@  static av_cold int vtenc_close(AVCodecContext *avctx)
 {
     VTEncContext *vtctx = avctx->priv_data;
 
-    pthread_cond_destroy(&vtctx->cv_sample_sent);
-    pthread_mutex_destroy(&vtctx->lock);
-
-    if(!vtctx->session) return 0;
+    if(!vtctx->session) {
+        pthread_cond_destroy(&vtctx->cv_sample_sent);
+        pthread_mutex_destroy(&vtctx->lock);
+        return 0;
+    }
 
     VTCompressionSessionCompleteFrames(vtctx->session,
                                        kCMTimeIndefinite);
     clear_frame_queue(vtctx);
+    pthread_cond_destroy(&vtctx->cv_sample_sent);
+    pthread_mutex_destroy(&vtctx->lock);
     CFRelease(vtctx->session);
     vtctx->session = NULL;