[vlc-devel] compat pthread_cancel

[Rémi Denis-Courmont]

Le samedi 11 juin 2011 11:18:30 Jean-Philippe André, vous avez écrit :
> > And then, either this whole thing makes no sense or it relies on
> > super-POSIX properties of Android libc that I don't know and thus
> > cannot review.
> 
> The only super-posix property I remember is that poll() returns whenever
> the process receives a signal. (ppoll does not exist)

I'm not sure what you mean. POSIX poll() certainly returns -1 on signal and 
sets EINTR. Unfortunately, it is not reliable for the reasons that lead to the 
invention of POSIX pselect() and Linux ppoll(). I don't see how Android be any 
better. This problem is intrinsic to the definitions of select(), poll() and 
of asynchronous signals.

> >  > Arf... What parts seem dubious?
> >
> > In my world:
> > * Signal handlers are (unfortunately) per-process, not per-thread.
> 
> Yes, unfortunately. This can indeed lead to issues.

I don't see any issue except for being a waste. And it makes me feel like the 
author of the code did not know what (s)he was doing.

> > * Setting thread keys to zero is pointless.
>
> Indeed, but not an issue.
>
> > * POSIX thread functions are not async-signal-safe, meaning they are not
> > defined within signal handlers.
>
> Thus many FIXME and 'Why?' comments^^
> Unfortunately I didn't find any other easily implementable solution that
> wouldn't be based on signals

Unless Android ensures that those functions are async-signal-safe, which it 
could even though POSIX does not, this does not work. If it does not work, it 
does not belong in the vlc.git.

> and that doesn't add yet another main thread watching the others.

With neither pselect() nor ppoll(), there is really not much practical use for 
signals in a multi-thread context. But this is moot: Within POSIX, there is 
not much a signal handler can do if its thread is blocked in, say, 
pthread_cond_(timed)wait(). Thus signals cannot be used for the purpose of 
cancelling a thread, unless you (can) make assumptions about the C run-time 
beyond POSIX promises.

> Maybe futexes are the way to go, but my knowledge of them is even worse
> than that of pthread^^

Obviously most preemptive implementations of POSIX threads use a reserved 
real-time signal to implement cancellation. But they rely on implementation-
dependent properties, such as atomic memory operations or async-signal-safe 
internal and proprietary functions.

I don't know Android libc, but I doubt this approach will ever work there.

> > * pthread_setcancelstate() is not a cancellation point.
> 
> OK, my bad. Slight misunderstanding/misreading of the API back then. Easy
> to remove the cancellation point here :)
> 
> >  I also don't understand where cancellation cleanup handlers
> > (vlc_cleanup_push and friends) are dealt with.
> 
> They are dealt with by the call to pthread_exit. If there's any
> vlc-specific task to execute, then yes, it's not being taken care of.

Ok. I did not realize that cleanup handlers were correctly handled even though 
cancellation was not.

> The code works, according to experimental testing, but it is not meant to
> be correct. That's one reason why I've never submitted it.


> Btw, cancellation on Android is probably one of the worst ideas we could
> ever have ;-)

It's the least common denominator of POSIX threads and Windows 5. Win32 
Asynchronous Procedure Calls are actually far more flexible than cancellation 
and signals, so POSIX was the portability bottleneck.

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis