[vlc-devel] [PATCH] VLCVideoUIView: remove racy assert

[Alexandre Janniaux]

Hi,

_enabled is actually protected by the main dispatch queue, so
turning it into an atomic just to check code manipulating the
vout_window object might not be the correct move.

The assert has been added after review because of the comment
around and was there to ensure there is no silent leak through
ARC because of vout window misuse.

Creating a dedicated testing vout_window implementation within
dedicated tests seems more appropriated for that use case.

Regards,
--
Alexandre Janniaux
Videolabs

On Mon, Jan 04, 2021 at 08:58:36AM +0100, Steve Lhomme wrote:
> It seems the comment is more related to the assert than resetting the value
> of _wnd.
>
> Couldn't this be an atomic boolean instead ? Because it seems the assert has
> some use. It's spotting improper use of the code, for example if it's
> refactored.
>
> On 2020-12-24 9:35, Alexandre Janniaux wrote:
> > _enabled is set asynchronously in the main thread whereas
> > detachFromParent is called from the thread closing the window, which is
> > usually different from the main thread. Checking _enabled here is racy
> > and could lead to triggering the assertion.
> > ---
> >   modules/video_output/apple/VLCVideoUIView.m | 1 -
> >   1 file changed, 1 deletion(-)
> >
> > diff --git a/modules/video_output/apple/VLCVideoUIView.m b/modules/video_output/apple/VLCVideoUIView.m
> > index 4acbfff8ac..3a4c749816 100644
> > --- a/modules/video_output/apple/VLCVideoUIView.m
> > +++ b/modules/video_output/apple/VLCVideoUIView.m
> > @@ -171,7 +171,6 @@
> >       /* The UIView must not be attached before releasing. Disable() is doing
> >        * exactly this asynchronously in the main thread so ensure it was called
> >        * here before detaching from the parent. */
> > -    assert(!_enabled);
> >       _wnd = NULL;
> >       vlc_mutex_unlock(&_mutex);
> >   }
> > --
> > 2.29.2
> >
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> >
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel