[vlc-devel] commit: Added ChromaCreate/Destroy (cosmetic)(Laurent Aimar )

[Remi Denis-Courmont]

On Mon, 30 Jun 2008 12:34:48 +0200, Pierre d'Herbemont
<pdherbemont at free.fr> wrote:
> Given that chroma are refcounted, it is probably not a good idea to do
the
> destructor that way. I think that you'd better use
> vlc_object_set_destructor().
> Else you might end up in using a destructed chroma->p_module.

This pattern is very questionable.

First, p_module should really _only_ ever be used by the "owner" of the
object, i.e. whatever thread invoked moodule_Need(). I see no reason why
third party threads would mess with the module field. In fact, even the
object itself should not touch the module field.

Second, you really want to call module_Unneed() from a thread that does not
have the module on its calls stack, and the obvious way to ensure this is
to use the "owner" thread. Consider this:

tA: caller creates object o1,
tA: caller loads a module m1 onto o1,
tA: m1's Open function starts a new thread tB,
tB: m1 creates object o2,
tB: m1 loads a module m2 onto o2,
tB: m2 yields object o1,

tA: caller releases o1,
tB: m2 releases o1,
tB: o2's destructor unloads m1,
tB: unmaps m1,
tB: m2 terminates,
tB: m2 unwinds to m1,
tB: SEGMENTATION FAULT because the code text section as been unmapped.

with Laurent's approach:
tA: caller creates object o1,
tA: caller loads a module m1 onto o1,
tA: m1's Open function starts a new thread tB,
tB: m1 creates object o2,
tB: m1 loads a module m2 onto o2,
tB: m2 yields object o1,

tA: callers unloads m1,
tA: m1's Close function kills and joins tB (blocks),
after a while:
tB: m2 releases o1,
tB: m2 terminates,
tB: m2 unwinds to m1,
tB: m1 task exits,
tA: m1's Close wakes up from joining tB,
tA: unmaps m1,
tA: caller releases o1.

Bottom line:
Never call module_Unneed() from a destructor.

For the similar reasons:
Never call vlc_thread_join() from a destructor.
(As I said a long time ago, the use of pthread_detach() is IMHO unsafe.)

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