[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1407309326.24324.274.camel@eris.loria.fr>
Date: Wed, 06 Aug 2014 09:15:26 +0200
From: Jens Gustedt <jens.gustedt@...ia.fr>
To: musl@...ts.openwall.com
Subject: Re: PATCH: don't call cleanup handlers after a regular
return from the thread start function
Am Dienstag, den 05.08.2014, 22:02 -0400 schrieb Rich Felker:
> > 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.
>
> This doesn't solve the problem. The problem is not the lifetime of the
> cancellation buffer (which doesn't even need to exist for call_once),
> but the fact that there's no way to track that the in-progress call
> was "finished" by longjmp so that another one can be started.
I don't know if this "solves" one of the problems related to that, but
it makes the code more robust, would enable us to give a decent
diagnostic before crashing and would probably ease debugging of such
crappy code.
> BTW a related issue is that recursive calls to call_once are
> unspecified: what happens if the init function passed to call_once
> itself calls call_once with the same once_flag object? :)
>
> I've reported the pthread_once issue here, and added a note on the
> other related items (recursive calls and pthread_exit) that came up
> later:
>
> http://austingroupbugs.net/view.php?id=863
Nice summary.
(I'd wish, WG14 had such a "bug tracker")
> > (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.)
>
> In principle, it's nice for a call_once like function not to require
> static storage. It means you can use it inside dynamically allocated
> objects for ctor-like purposes where the ctor isn't always needed and
> can be run lazily on the first operation that needs it. Of course
> call_once is useless for this since it doesn't allow passing an
> argument to the function. So I have no objection to requesting that
> C11 be amended to require static storage for once_flag.
Yes such a dynamic initialization tool would be nice (P99 has it) and
I agree that once_flag (or pthread_once) isn't it.
I'd also just go for the "static" because POSIX has this, and I don't
think that it is a good idea to get the semantics of the different
control structures too much out of sync between the two standards.
> > followed by a fight to get the requirement for static
> > allocation and interdiction of longjmp and thrd_exit into the
> > specification of call_once.
>
> I don't think it will be much of a fight.
sometimes I am very much surprised what proposals are vetoed
a valid initial state of statically allocated atomics or mutexes is
one of those that I already tried to convince people, meeting a
surprisingly strong resistance
> BTW while you're at it,
> there might be a number of additional issues -- like what happens if
> atexit handlers call thrd_exit, what happens if tss dtors longjmp,
> etc. As far as I can tell, none of this is properly specified and IMO
> it should all be explicit UB.
Yes.
I just also added recursive calls to call_once on the same object to
my list.
> > > - 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.
>
> It's not possible to mix this with cancellation (or with C++
> exceptions, for that matter). But as long as neither "unwinds" across
> the other, you're fine. This is a restriction you need to follow and
> be aware of anyway. It's not something we can "work around" except by
> using the same underlying implementation for all three, and that's
> undesirable for various reasons (one of which is that people should
> not be relying on it).
>
> Even if you did want to support this, making it work is a lot harder
> than allowing longjmp out of pthread_once/call_once. The latter are
> special cases where there's at most one cancellation cleanup layer and
> it's implementation-internal. The general case has arbitrary
> cancellation cleanup buffers at different call frame contexts, and
> they're most certainly not static.
Perhaps I wouldn't like to support this, but make the library more
robust against such issues as said above. Give diagnostics ("cleanup
handler stack not empty after thread termination") and ease debugging.
If, as we seem both to intend, all this ends up to be clearly UB,
nothing prevents us from being nice if this happens. What do you think
of my initial patch (which is not very intrusive), but instead of
clearing `cancelbuf` to call abort?
> > (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.)
>
> Yes, that's an unfortunate issue which I should fix. But making the
> once_flag a big structure rather than a single int (which will surely
> disagree with the glibc ABI) is not the solution. The solution is just
> to have a potential-waiters flag in the single int, rather than a
> waiter count. The incidence of contention is so low that it doesn't
> matter if it results in a rare excess syscall, especially since once
> calls are once-per-program-invocation.
Sure, as I said changing the ABI for pthread_once is out of the
question.
Yes, I agree that this is a minor "problem", if at all.
You could even have a counter in there, the state only needs 2 bit,
the rest could be used as a wait counter.
One idea would be to have that int as a counter, and to have special
values of the counter reflect the state. 0 would be unitialized, -1
completely initialized, all other would be the number of waiters +1.
> > > 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.
>
> No it won't. Cancellation is unusable with almost all existing code
> (and almost all new code). To use cancellation, the target thread must
> be running code which was written to be aware of the possibility of
> being cancelled. Thus, it's not going to "just happen".
With "will happen" I mean that code will just do it, perhaps by some
tool implemented with pthreads shooting at a C thread.
> > too late, done already, and it is not too ugly, either
>
> What's already done?
the call_once implementation with statically allocated state
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 ::
Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
