[vlc-devel] commit: Implement access_GetParentInput and demux_GetParentInput and use. ( Pierre d'Herbemont )

[Pierre d'Herbemont]

On Aug 26, 2009, at 5:12 PM, Rémi Denis-Courmont wrote:

> Le mercredi 26 août 2009 17:42:01 Pierre d'Herbemont, vous avez  
> écrit :
>> First, demux and access don't properly implement refcounting. If
>> someone is holding a reference to it, its data might get destroyed,
>> and lead to crash for free.
>
> How so? To my knowledge *none* of the demux_t and access_t specific  
> members
> are thread-safe. So none of them should be accessed from
> vlc_object_find(_name).

Are they really?

demux_IsPathExtension()
{
     const char *psz_ext = strrchr ( p_demux->psz_path, '.' );
     if( !psz_ext || strcasecmp( psz_ext, psz_extension ) )
         return false;
}

demux_Delete()
{
     ...
     free( p_demux->psz_path );
     ...
     vlc_object_release( p_demux );
}

So if we do a:
vlc_object_hold(p_demux)
and then one of the owner (the main one?) gets to delete this object.
demux_IsPathExtension() will crash.

Are we not supposed to do that? Fine, but don't let it compile without  
warning.

> So none of them need to be reference-counted.

  So, if they don't need to be refcounted and doesn't support  
refcounting, vlc_object_hold() should be disabled on those objects then.

Either you have:
- one owner and no refcounting.
- multiple owner and a way to interrupt the service (The so called  
Delete), in this case you have to ensure that accessors will not  
crash, or will return something coherent.

>> (They should implement a destructor or set
>> at list its destroyed instance variable to NULL).
>
> A destructor is only useful when you have some type-specific data to  
> be
> accessed through the object tree. Then you need to destroy it only  
> when the
> reference count drops to zero. Most types don't, so a type-specific  
> destroy
> function is sufficient, which is called by the "owner" of the object.


They are multiple owners when you do refcounting.

Pierre.