[vlc-devel] [PATCH 0/1] dshow: Fix assertion when stopping

[Hugo Beauzée-Luyssen]

On Thu, Jun 1, 2017, at 06:10 PM, Rémi Denis-Courmont wrote:
> Le torstaina 1. kesäkuuta 2017, 13.55.24 EEST Hugo Beauzée-Luyssen a
> écrit :
> > > > > Isn´t FindDevices() potentially called from the GUI thread?
> > > > > Isn´t the GUI thread entered in an STA?
> > > > 
> > > > Only the Qt GUI calls CoInitializeEx (as far as interfaces are
> > > > concerned, obviously)
> > > 
> > > Yes but isn´t the Qt Win32 port also entering an apartment internally,
> > > and isn´t that unfortunately an STA?
> > 
> > It does enter an STA, but it immediately releases it once it's done with
> > handling file associations.
> 
> Are you sure that this is warranted by Qt API? That looks like a version-
> dependent implementation detail. I was not even aware that we managed 
> associations through Qt.
> 

I was refering to VLC's code, in SPrefsPanel::assoDialog()
I'm not aware of any thread model specification on Qt's side

> > > The enumeration callbacks indeed seem fine. But what about the access
> > > callbacks? What happens if Open() and Read(), or Open() and Close() are
> > > serialized yet called in different threads?
> > > 
> > > (...)
> > > 
> > > > Given that you say it would be safe if (...), and that this condition is
> > > > met, I'm not sure why this patch is wrong. What would be the correct
> > > > solution in your opinion?
> > > 
> > > AFAICT, either the DirectShow code has a thread keeping the MTA alive for
> > > the
> > > lifetime of the access/access_demux instance, or we ensure that another
> > > thread
> > > takes care of the same (e.g. in Win32 specific core code).
> > 
> > The MTA gets released by the dshow module from its Close() function, but
> > if I understand you correctly, you're worry is that Open and Close won't
> > be called from the same thread, right?
> > To be honest I'd have assumed it was always the case
> 
> There is no such warranty. I don´t recall any specific where Open and
> Close 
> would be on different threads. But I wouldn´t be surprised if it was
> possible 
> in other call access instantiation sites than the input master or input 
> slaves.
> 
> It is definitely possible for Read/Control to run in different thread
> than 
> Open/Close, e.g. with prefetch. Which means you can´t assume that the MTA
> is 
> entered during Read or Control, unless you do so explicitly.
> 

The open/close part is surprising to me, but definitely not for
Read/Control.

> And then, there´s the problem that another module in the same thread
> might 
> attempt to enter an STA. A module should never leak its apartmenet to its 
> caller, IMO.
> 

I guess we could also re-enter MTA from all public dshow "entrypoints",
this would indeed prevent us from leaking the STA/MTA to the input
thread, until dshow terminates.

I'd still be in favor or going away from MTA in the case of dshow, I'll
craft a patch with both approches merged.

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr