[vlc-devel] [PATCH] vout/macosx: simplify close and make sure all AppKit objects are released on the main thread

[Felix Paul Kühne]

Hello again,

> On 18. Sep 2019, at 09:08, Thomas Guillem <thomas at gllm.fr> wrote:
> 
> 
> 
> On Tue, Sep 17, 2019, at 22:12, Felix Paul Kühne wrote:
>> From: Felix Paul Kühne <felix at feepk.net>
>> 
>> Note that this does _not_ solve #22766.
>> ---
>> modules/video_output/macosx.m | 29 +++++++++++++----------------
>> 1 file changed, 13 insertions(+), 16 deletions(-)
>> 
>> diff --git a/modules/video_output/macosx.m b/modules/video_output/macosx.m
>> index 6290bbf763..5f618a0716 100644
>> --- a/modules/video_output/macosx.m
>> +++ b/modules/video_output/macosx.m
>> @@ -265,21 +265,20 @@ static void Close(vout_display_t *vd)
>>         [sys->glView setVoutDisplay:nil];
>> 
>>         var_Destroy (vd, "drawable-nsobject");
>> -        if ([(id)sys->container 
>> respondsToSelector:@selector(removeVoutSubview:)])
>> -        /* This will retain sys->glView */
>> -            [(id)sys->container 
>> performSelectorOnMainThread:@selector(removeVoutSubview:)
>> -                                                 withObject:sys->glView
>> -                                              waitUntilDone:NO];
>> -
>> -        /* release on main thread as explained in Open() */
>> -        [(id)sys->container 
>> performSelectorOnMainThread:@selector(release)
>> -                                             withObject:nil
>> -                                          waitUntilDone:NO];
>> -        [sys->glView 
>> performSelectorOnMainThread:@selector(removeFromSuperview)
>> -                                      withObject:nil
>> -                                   waitUntilDone:NO];
>> -
>>         var_Destroy(vlc_object_parent(vd), "macosx-glcontext");
>> +
>> +        dispatch_sync(dispatch_get_main_queue(), ^{
>> +            if ([(id)sys->container 
>> respondsToSelector:@selector(removeVoutSubview:)]) {
>> +                /* This will retain sys->glView */
>> +                [(id)sys->container removeVoutSubview:sys->glView];
>> +            }
>> +
>> +            /* release on main thread as explained in Open() */
>> +            [(id)sys->container release];
>> +            [sys->glView removeFromSuperview];
>> +            [sys->glView release];
>> +        });
>> +
>>         if (sys->vgl != NULL)
>>         {
>>             vlc_gl_MakeCurrent(sys->gl);
> 
> What will MakeCurrent do if glView is released ? 
> I think this code should be executed before glView release.

Absolutely valid remark. In fact, the dispatch block should conclude the Close function just before freeing sys.

Best regards,

Felix