Explaining cond var destroy [Re: [musl] C threads, v3.0]

[Rich Felker <dalias@libc.org>]

On Mon, Aug 04, 2014 at 11:30:03AM +0200, Jens Gustedt wrote:
> diff --git a/src/thread/cnd_destroy.c b/src/thread/cnd_destroy.c
> new file mode 100644
> index 0000000..7d6f3a1
> --- /dev/null
> +++ b/src/thread/cnd_destroy.c
> @@ -0,0 +1,21 @@
> +#include "pthread_impl.h"
> +#include <threads.h>
> +
> +/* The behavior of cnd_destroy is undefined if cnd is still in
> +   use. The choice for pthread_cond_destroy in that situation is to
> +   wake up all users before destroying. I am not sure that we should
> +   do it like that here, too. Alternatives would be:
> +   - complain by using perror or equivalent
> +   - assert that there is no waiter
> +   - abort when there is a waiter
> +   - do nothing
> +   */
> +void cnd_destroy(cnd_t *c)
> +{
> +	int cnt;
> +	c->_c_destroy = 1;
> +	if (c->_c_waiters)
> +		__wake(&c->_c_seq, -1, 0);
> +	while ((cnt = c->_c_waiters))
> +		__wait(&c->_c_waiters, 0, cnt, 0);
> +}

The above comment is incorrect; I'll try to explain. At least for
POSIX cond vars, per POSIX, at least one thread that has called
pthread_cond_[timed]wait ceases to be a waiter as soon as
pthread_cond_signal is called, and all threads which have called
pthread_cond_[timed]wait ceast to be waiters as soon as
pthread_cond_broadcast is called. This means that, if a thread calling
pthread_cond_signal or pthread_cond_broadcast has updated the
predicate such that no threads will retry pthread_cond_[timed]wait or
remain as waiters, it may IMMEDIATELY call pthread_cond_destroy
without violating the constraint that pthread_cond_destroy can only be
called when there are no waiters.

Since waiters have additional work to do on the memory associated with
the pthread_cond_t object after the futex wait completes, and since we
do not want to force them to wake and finish this work as part of
pthread_cond_signal/broadcast (this would be expensive on every
signal), I've put the code to finish waiting for the waiters to wake
up in pthread_cond_destroy.

If you think this is a bad idea, I'd be willing to hear alternate
ideas. I'm not really happy with the cond var implementation (if
nothing else, the sequence number thing is an ugly hack and not 100%
robust, I think) and at some point I'd like to redesign it.

Rich