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

[Rémi Denis-Courmont]

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.

> 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.

-- 
雷米‧德尼-库尔蒙
http://www.remlab.net/