Detect some attempts to destroy a condvar with waiters.

[markj]

Summary

The check is racy if the associated mutex is not held, but in that case
a race is present anyway, so the situation is not made worse.

Add a couple of tests, one which direct checks that we return EBUSY as
desired, and one which verifies that one can destroy a condvar
immediately after waking up sleepers. This will help catch bugs like
the one referenced in r283250 (there it was the kernel's internal
condvar implementation).

[jhb]

This looks right to me. The only change I had locally is that I had added a helper function in thr_umtx.h since that seemed to match the style of other code in libthr:

static inline int
_thr_ucond_has_waiters(struct ucond *cv)
{
    return (cv->c_has_waiters != 0);
}

and then used that instead of looking at cvp->kcond directly. I don't mind either way though.

[markj]

In D19496#417290, @jhb wrote:

This looks right to me. The only change I had locally is that I had added a helper function in thr_umtx.h since that seemed to match the style of other code in libthr:

static inline int
_thr_ucond_has_waiters(struct ucond *cv)
{
    return (cv->c_has_waiters != 0);
}

and then used that instead of looking at cvp->kcond directly. I don't mind either way though.

I considered this, but note that the tests are different depending on the condvar type, and there is existing code in thr_cond.c and thr_umtx.c that checks the field values directly.

[jhb]

It was only a minor suggestion as at the time it seemed more consistent. In my patches I hadn't handled the pshared case yet either fwiw.

[ngie]

Two questions:

• is there a test case for the EBUSY behavior?
• is there a reason why one of the test cases was ifdef’ed FreeBSD, but the other wasn’t?

[markj]

In D19496#417321, @ngie wrote:

Two questions:

• is there a test case for the EBUSY behavior?

Yes, destroy_busy().

• is there a reason why one of the test cases was ifdef’ed FreeBSD, but the other wasn’t?

The code change implements non-standard behaviour, and the test for that behaviour may fail on conforming implementations of the pthread API (like NetBSD's). The other test is for behaviour that is specifically permitted by the standard, so there's no reason to ifdef.

[ngie]

I missed the test for EBUSY. Thank you for pointing that out!

CCing @jilles, just in case they want to chime in. Does this also need a doc update?

[ngie]

Answering myself: no, it’s already documented.