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

[David Fuhrmann]

> Am 17.09.2019 um 22:12 schrieb Felix Paul Kühne <fkuehne at videolan.org>:
> 
> 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(), ^{

Hi Felix,

As briefly discussed over chat, my fear is that this switch from async to sync deadlocks once closing VLC with a running video.

In fact, I can easily reproduce this with your patch applied, just try to quit VLC while a video is running:

Mainloop has been stopped already here:
[00007f8cfe408360] main libvlc debug: exiting
[00007f8cfe408360] main libvlc debug: removing all interfaces
[00007f8cfe6168c0] main interface debug: removing module "macosx"
[00007f8cfe6168c0] macosx interface debug: Closing macosx interface
...
[00007f8cff03d800] main decoder debug: removing module "videotoolbox"
[00007f8cfe42cc90] main player debug: saving a free vout
—> Only here the vout closes, and thus the vout thread deadlocks.

Also, I do not really see what you actually want to fix here, even if not optimal the original code seems to work fine.

BR. David


> +            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);
> @@ -294,8 +293,6 @@ static void Close(vout_display_t *vd)
>             vlc_object_delete(sys->gl);
>         }
> 
> -        [sys->glView release];
> -
>         free (sys);
>     }
> }
> -- 
> 2.20.1 (Apple Git-117)
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel