[vlc-devel] commit: objects: Call vlc_object_join()automatically from vlc_object_destructor() if needed. (Pierred'Herbemont )

[Rémi Denis-Courmont]

Le Tuesday 25 March 2008 19:16:26 Pierre d'Herbemont, vous avez écrit :
> > EDEADLK is a MAY fail error, not a SHALL fail error. There is no
> > obligation for pthread implementations to handle this properly.
>
> Can we agree that sane pthread implementation do that? (like
> vlc_assert_locked() and such).

I don't agree.

First, debug VLC builds use error-checking mutexes, so that means 
vlc_assert_locked() SHALL fail (and abort) when appropriate. We have no such 
provisions for threads. In fact, I doubt that POSIX defines error-checking 
threads anyway or something equivalent.

Second, as regards debug assertion statements, avoiding false negatives is 
critical. Avoiding false positives is more of a best-effort - there are not 
always avoidable anyway. In other words, even if we had no warranty that 
vlc_assert_locked() would detect always errors, it would still be "better 
than nothing".

In the pthread_join() case, we need a warranty that dead locks are detected. 
We don't have it. Hence, it is broken. Full point.

> > Besides, calling
> > pthread_join() multiple times against the same thread is explicitly
> > undefined.
>
> Where would we do that? (vlc_thread_join set b_thread to false).

There are two ways to cleanup a POSIX thread:
- join from another thread, must be done once and only once, or
- detach the thread (possibly from within itself).

For any given thread, you have to make sure either of these happens, 
predictably. If you want vlc_object_destroy() to be callable both from inside 
the thread and from outside, and otherwise assuming proper locking and 
serialization:

if (pthread_equal (pthread_self (), obj->threadid))
	pthread_detach (obj->threadid);
else
	pthread_join (obj->threadid, NULL);

I haven't checked how Win32 handles this.


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