[vlc-devel] Handling deadlock between display:Open() and vout_window_ReportSize()
remi at remlab.net
Sat Dec 14 08:47:03 CET 2019
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
> > > 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.
More information about the vlc-devel