Have pthread_cond_destroy() return EBUSY if the condvar has waiters.

This is not required of a compliant implementation, but it's easy to
check for and helps improve compatibility with other common
implementations.  Moreover, it's consistent with our
pthread_mutex_destroy().

PR:		234805
Reviewed by:	jhb, kib, ngie
MFC after:	2 weeks
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D19496
This commit is contained in:
Mark Johnston 2019-03-08 21:07:08 +00:00
parent 673544c3dd
commit b16150ea90
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=344935
2 changed files with 106 additions and 5 deletions

View File

@ -493,6 +493,51 @@ ATF_TC_BODY(bogus_timedwaits, tc)
PTHREAD_REQUIRE(pthread_mutex_unlock(&static_mutex));
}
#ifdef __FreeBSD__
static void *
destroy_busy_threadfunc(void *arg)
{
PTHREAD_REQUIRE(pthread_mutex_lock(&mutex));
share = 1;
PTHREAD_REQUIRE(pthread_cond_broadcast(&cond));
PTHREAD_REQUIRE(pthread_cond_wait(&cond, &mutex));
PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
return NULL;
}
ATF_TC(destroy_busy);
ATF_TC_HEAD(destroy_busy, tc)
{
atf_tc_set_md_var(tc, "descr", "Checks non-standard behaviour of "
"returning EBUSY when attempting to destroy an active condvar");
}
ATF_TC_BODY(destroy_busy, tc)
{
pthread_t thread;
PTHREAD_REQUIRE(pthread_mutex_init(&mutex, NULL));
PTHREAD_REQUIRE(pthread_cond_init(&cond, NULL));
PTHREAD_REQUIRE(pthread_mutex_lock(&mutex));
PTHREAD_REQUIRE(pthread_create(&thread, NULL, destroy_busy_threadfunc,
NULL));
while (share == 0) {
PTHREAD_REQUIRE(pthread_cond_wait(&cond, &mutex));
}
PTHREAD_REQUIRE_STATUS(pthread_cond_destroy(&cond), EBUSY);
PTHREAD_REQUIRE(pthread_cond_signal(&cond));
PTHREAD_REQUIRE(pthread_cond_destroy(&cond));
PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
PTHREAD_REQUIRE(pthread_join(thread, NULL));
PTHREAD_REQUIRE(pthread_mutex_destroy(&mutex));
}
#endif
static void
unlock(void *arg)
{
@ -547,6 +592,49 @@ ATF_TC_BODY(destroy_after_cancel, tc)
PTHREAD_REQUIRE(pthread_mutex_destroy(&mutex));
}
static void *
destroy_after_signal_threadfunc(void *arg)
{
PTHREAD_REQUIRE(pthread_mutex_lock(&mutex));
share = 1;
PTHREAD_REQUIRE(pthread_cond_broadcast(&cond));
PTHREAD_REQUIRE(pthread_cond_wait(&cond, &mutex));
PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
return NULL;
}
ATF_TC(destroy_after_signal);
ATF_TC_HEAD(destroy_after_signal, tc)
{
atf_tc_set_md_var(tc, "descr", "Checks destroying a condition variable "
"immediately after signaling waiters");
}
ATF_TC_BODY(destroy_after_signal, tc)
{
pthread_t thread;
PTHREAD_REQUIRE(pthread_mutex_init(&mutex, NULL));
PTHREAD_REQUIRE(pthread_cond_init(&cond, NULL));
PTHREAD_REQUIRE(pthread_mutex_lock(&mutex));
PTHREAD_REQUIRE(pthread_create(&thread, NULL,
destroy_after_signal_threadfunc, NULL));
while (share == 0) {
PTHREAD_REQUIRE(pthread_cond_wait(&cond, &mutex));
}
PTHREAD_REQUIRE(pthread_cond_signal(&cond));
PTHREAD_REQUIRE(pthread_cond_destroy(&cond));
PTHREAD_REQUIRE(pthread_mutex_unlock(&mutex));
PTHREAD_REQUIRE(pthread_join(thread, NULL));
PTHREAD_REQUIRE(pthread_mutex_destroy(&mutex));
}
ATF_TC(condattr);
ATF_TC_HEAD(condattr, tc)
{
@ -577,7 +665,11 @@ ATF_TP_ADD_TCS(tp)
ATF_TP_ADD_TC(tp, cond_timedwait_race);
ATF_TP_ADD_TC(tp, broadcast);
ATF_TP_ADD_TC(tp, bogus_timedwaits);
#ifdef __FreeBSD__
ATF_TP_ADD_TC(tp, destroy_busy);
#endif
ATF_TP_ADD_TC(tp, destroy_after_cancel);
ATF_TP_ADD_TC(tp, destroy_after_signal);
ATF_TP_ADD_TC(tp, condattr);
return atf_no_error();

View File

@ -166,17 +166,26 @@ _pthread_cond_destroy(pthread_cond_t *cond)
error = 0;
if (*cond == THR_PSHARED_PTR) {
cvp = __thr_pshared_offpage(cond, 0);
if (cvp != NULL)
__thr_pshared_destroy(cond);
*cond = THR_COND_DESTROYED;
if (cvp != NULL) {
if (cvp->kcond.c_has_waiters)
error = EBUSY;
else
__thr_pshared_destroy(cond);
}
if (error == 0)
*cond = THR_COND_DESTROYED;
} else if ((cvp = *cond) == THR_COND_INITIALIZER) {
/* nothing */
} else if (cvp == THR_COND_DESTROYED) {
error = EINVAL;
} else {
cvp = *cond;
*cond = THR_COND_DESTROYED;
free(cvp);
if (cvp->__has_user_waiters || cvp->kcond.c_has_waiters)
error = EBUSY;
else {
*cond = THR_COND_DESTROYED;
free(cvp);
}
}
return (error);
}