Re: PATCH: don't call cleanup handlers after a regular return from the thread start function

[Jens Gustedt <jens.gustedt@inria.fr>]

[-- Attachment #1: Type: text/plain, Size: 5551 bytes --]

Am Dienstag, den 05.08.2014, 17:48 -0400 schrieb Rich Felker:
> On Tue, Aug 05, 2014 at 11:05:41PM +0200, Jens Gustedt wrote:
> > It is not explicitly forbidden and I think this is sufficient to allow
> > for it.
> 
> Actually I don't think it's possible to implement a call_once that
> supports longjmp out without a longjmp which does unwinding,

simple longjmp, perhaps not, longjmp that is followed by thread
termination, yes. I just have done it, so it is possible :)

As I mentioned earlier, you can do this by having everything
statically allocated. I am using a struct for once_flag that has the
`int` for control, another `int` for waiters and a `struct __ptcb` for
the call to _pthread_cleanup_push. call_once is just a copy of
pthread_once, just using these fields of the once_flag, instead of the
local variables.

(Strictly spoken, once_flag is not required to be statically
allocated, as is the explicit requirement for pthread_once_t. Yet
another omission of C11, it is on my list for addition.)

> and I
> don't think it's the intent of WG14 to require unwinding. The
> fundamental problem is the lack of any way to synchronize "all
> subsequent calls to the call_once function with the same value of
> flag" when the code to update flag to indicate completion of the call
> is skipped.

sure

> Do you see any solution to this problem? Perhaps a conforming
> implementation can just deadlock all subsequent calls, since (1)
> calling the function more than once conflicts with the "exactly once"
> requirement, and (2) the completion of the first call, which
> subsequent calls have to synchronize with, never happens.

one part of solution is the "static" implementation as mentioned
above, followed by a fight to get the requirement for static
allocation and interdiction of longjmp and thrd_exit into the
specification of call_once.

> - In the case of longjmp, it's already dangerous to longjmp across
>   arbitrary code. Unless you know what code you're jumping over is
>   doing or have a contract that it's valid to jump over it, you can't
>   use longjmp.

sure, but just consider the nice tool-library that implements
try-catch sort of things with setjmp/longjmp.

> > For the case of pthread_once or call_once, we could get away with it
> > by having the `struct __ptcb` static at that place. The caller of the
> > callback holds a lock at the point of call, so there would be no race
> > on such a static object.
> > 
> > I will prepare a patch for call_once to do so, just to be on the safe
> > side. This would incur 0 cost.
> 
> The cost is encoding knowledge of the cancellation implementation at
> the point of the call, and synchronizing all calls to call_once with
> respect to each other, even when they don't use the same once_flag
> object.

No, I don't think so, and with what I explained above this is perhaps
a bit clearer. The only thing that this does is making all the state
of a particular call to call_once statically allocated.

(There is even less interaction between calls to call_once with
different objects, than we have for the current implementation of
pthread_once. That one uses a `static int waiters` that is shared
between all calls.)

> call_once does not have specified behavior if the callback is
> cancelled (because, as far as C11 is concerned, cancellation does not
> even exist). Thus there is no need for a cleanup function at all.

After digging into it I don't agree. In a mixed pthread/C thread
environment cancellation will eventually happen, so we need a cleanup
function to avoid deadlocks.

> I think we should wait to do anything unneccessary, ugly, and/or
> complex here unless the next issue of POSIX specifies a behavior for
> this case.

too late, done already, and it is not too ugly, either

> > The same could be used for pthread_once.
> 
> Above analysis about longjmp being impossible to support without
> deadlock or unwinding in longjmp also applies to pthread_once. I
> suspect, if longjmp is implicitly allowed, this is just an oversight
> and one which will be corrected.

right

also introducing the same idea as above would be an ABI change, so
let's not touch this

> We're talking about a return statement in between the
> pthread_cleanup_push and pthread_cleanup_pop calls in a particular
> block context. Third-party library code does not magically insert such
> a statement in your code. So I don't understand the case you have in
> mind.

No it is not only return, it also is longjmp. As mentioned above,
consider a pure C11 context where I use a tool box that implements
try-catch things by means of setjmp/longjmp.

Then I use another, independent library that uses callbacks, that can
be graphics, OpenMp, libc, whatever. And perhaps on some POSIX system
it uses pthread_cleanup_push under the hood.

It will be hard to ensure that both tools when used together on an
arbitrary platform that implements them will not collide with the
requirement not to use longjmp across a pthread_cleanup context.

(BTW, in the musl timer code I just detected a nasty usage of longjmp
from within a cleanup handler. That one is ugly :)

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]