[vlc-devel] Handling deadlock between display:Open() and vout_window_ReportSize()

[Alexandre Janniaux]

Hi,

On Sat, Dec 14, 2019 at 09:47:03AM +0200, Rémi Denis-Courmont wrote:
> Le perjantaina 13. joulukuuta 2019, 19.23.25 EET Alexandre Janniaux a écrit :
> > On Fri, Dec 13, 2019 at 07:06:35PM +0200, Rémi Denis-Courmont wrote:
> > > Le perjantaina 13. joulukuuta 2019, 18.54.22 EET Alexandre Janniaux a
> écrit :
> > > > On Fri, Dec 13, 2019 at 06:08:38PM +0200, Rémi Denis-Courmont wrote:
> > > > > 	Hi,
> > > > >
> > > > > There's an obvious lock inversion in that patch that will break all
> > > > > platforms.
> > > >
> > > > For the inversion lock, the inversion is obvious in the
> > > > current codebase, but is impossible if you enable with the
> > > > right state because of the ordering of events and state, as
> > > > far as I checked. It's not impossible that it's imperfect
> > > > though, I would have submitted a patch if I were sure of
> > > > what I was doing.
> > >
> > > We use strict lock ordering in VLC like pretty much everywhere else.
> > > Anything else will confuse developers, reviewers, sanitizers and
> > > analyzers too much to bare.
> >
> > I agree, that's why I'm posting, do you have any other idea?
>
> Other idea implies that there are any ideas. This patchset, behind the
> apparent lock inversion, introduces a data race on sys->display. It's broken.

I don't see the data race, and again, it's not a patchset.
I'm looking for advice or solution, not review, if it wasn't
clear by the missing [PATCH] in the title.

> > To sum up the issue, we shouldn't try locking the display
> > lock and resizing the display at all if it doesn't exist yet,
>
> No. That's supposed to work, as are all other aout/vout user-driven changes.
>
> If your display plugin can't handle it, it's broken, not the core. And indeed,
> it seems like the usual plugin misdesign where it sends an event to itself.

Currently the display module creates its UIView from
window dummy, and explicitly requires that it is dummy.
It also creates the EAGL context itself.

In my refactor, I have a window module and an opengl module
but I use the same display as on Linux and Android. The iOS
modulee is completely removed.

As it is two different modules, I don't understand your point
about sending event to itself. Here you maybe mean itself as
the UI loop, which is not was is happening. There is a clear
separation between the OpenGL provider module and the vout
window module. Please check the chaining of events that I
described in the first post to understand, if you want to
get involved in this discussion.

Regards,
--
Alexandre Janniaux
Videolabs