[vlc-devel] [PATCH 1/8] Extensions: public include

[Rémi Denis-Courmont]

Le vendredi 25 décembre 2009, Jean-Philippe André a écrit :
> > > +#define extension_UnsetDialogProvider(a) \
> > > +        __extension_UnsetDialogProvider( VLC_OBJECT(a) )
> > > +static inline void __extension_UnsetDialogProvider( vlc_object_t
> > > *p_this ) +{
> > > +    var_Destroy( p_this, "dialog-provider" );
> > > +}
> >
> > Seems unsafe. The caller does not know when the provider is
> > unreferenced. Also, if the variable was created more than once,
> > this won't even unregister the pointer.
>
> Even if the "Set" function is meant to be called only once?
> Otherwise, we can set the variable to NULL, and then destroy it
> (looks strange).

That's not the point. You need to make sure that no thread uses the 
former value of the variable after you destroyed it:

Thread A: var_Set
 ...
Thread B: var_Get
Thread A: var_Destroy
Thread A: destroys internal data
Thread B: tries to use the data *boom*

Address variables should really only be read through -synchronous- 
callbacks (as the dialogs currently do), not through getters. That's 
the only sane way w.r.t. the provider's life cycle.

And then, there will be a problems with dialogs if they do not block the 
caller thread. See the FIXME in dialog_ProgressCreate().

-- 
Rémi Denis-Courmont
http://www.remlab.net/