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

[Pierre d'Herbemont]

On Jun 15, 2008, at 5:47 PM, Rémi Denis-Courmont wrote:

> Le dimanche 15 juin 2008 15:55:04 Pierre d'Herbemont, vous avez  
> écrit :
>>> You just
>>> cannot destroy the playlist until the directory plugin has not
>>> completed
>>> reading its directory (which is a set of non-blocking operations).
>>
>> Actually it doesn't work, and you'll end up in leaking some input
>> item, if you kill libvlc during directory processing. I am not sure
>> about why.
>
> 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).

The rest of the code doesn't support
>

> Of course, there is a problem with the circular holding pattern,  
> even if it is
> time-bound. If an "owner" releases an object while that object is  
> held by its
> own child, when the child release the object, it accidentally  
> destroys its
> parent. That's why you've added the pthread_detach hack.

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

I suspect this one:
http://git.videolan.org/?p=vlc.git;a=commitdiff;h=a9565f33d3e6742d5a18c8a0e6b8d811d95cc885
But it was mostly reverted.

> 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.


> The real problem here, is that merging vlc_object_release and
> vlc_object_destroy was a bad idea. The "owner" of an object should  
> not rely
> on vlc_object_release, it should call vlc_object_destroy, which  
> should _wait_
> until all references are gone. To put it another way, the  
> pthread_detach()
> hack cannot ever work properly.

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.

Pierre.