[vlc-devel] commit: input: Remove p_playlist gc signaling now that the playlist is aware of vlc_InputSelectedStreamChanged . (Pierre d'Herbemont )

[Rémi Denis-Courmont]

Le dimanche 15 juin 2008 19:49:07 Pierre d'Herbemont, vous avez écrit :
> > What's the point of using object reference counting if you cannot
> > hold objects
> > anyway??? Besides, the playlist is supposed to wait for the input to
> > terminate, which is supposed to wait for the demux to terminate. If
> > you
> > intend to have a strictly hierarchical objects handling, then
> > references
> > really are useless.
>
> IMO, the problem arise because vlccore wasn't designed with reference
> counting in mind. (The vlc_object_destroy() function is exactly that).

There is no need for reference counting if only hierarchical accesses are 
allowed. The main use of reference counting in a threaded system is precisely 
to allow random objects to access random others. If children cannot reference 
their parents, I don't really see the use of reference counting.

> May I ask you exactly about what hack? pthread_detach: object_detach,
> object_kill?

There is only one place that I know using *pthread_detach*. This can only 
happen if a thread tries to destroy itself, which sounds wrong.

Now consider:

A creates B
A creates a thread through B
B creates C
C yields B
A releases B

And later:

C releases B

Once A releases B, B and C are completely isolated. Any cleanup procedure 
within A, that depended on B being destroyed will fail. pthread_detach is 
just one particular case of that more general problem. When A really wants to 
cleanup/destroy B, it should not simply release it, it should wait for every 
reference to go way, then destroy it.

As far as I can tell, there is absolutely no way to prevent this unless we 
forbid holding any object which is not one's direct child. And if we do that, 
reference counting is anyway useless, since there can only ever be one entity 
holding any given object.

> > Unfortunately, if
> > the "owner" happens to be libvlc, from libvlc_destroy(), then you
> > may destroy
> > critical core resources such as the playlist (which causes an
> > assertion with
> > the playlist_item reference checks), or worse, the messages bank,
> > the modules
> > bank, and finally the threads and objects subsystem (these will cause
> > crashes).
>
> I don't understand your point. If libvlc has a reference to those
> objects, they shouldn't be destroyed before libvlc has done with them.

Once libvlc has released an object, it has no way to know if the object has 
been destroyed, or if it is still held by yet another object. So LibVLC may 
endup reaching the "critical" late stages of instance clean up while some 
objects/threads are still active. That's why we still have input item leaks 
in fact.

> Well, no, because you loose the concept of refcounting. The whole
> purpose of refcounting is to be able to seemlessly switch owners. What
> needs to be fixed are libvlc objects. They need to properly destroy
> themselves, in the right order. Obviously the big playlist object is
> not doing that properly.


> Also, what you want is not vlc_object_destroy(). It's
> libvlc_playlist_stop() which would block and wait until playlist has
> stopped and released all its input.

This issue is not playlist-specific. It is present in anything that requires a 
well defined cleanup order.

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