svn commit: r344935 - in head: contrib/netbsd-tests/lib/libpthread lib/libthr/thread

[Mark Johnston]

Author: markj
Date: Fri Mar  8 21:07:08 2019
New Revision: 344935
URL: https://svnweb.freebsd.org/changeset/base/344935

Log:
  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

Modified:
  head/contrib/netbsd-tests/lib/libpthread/t_cond.c
  head/lib/libthr/thread/thr_cond.c

Modified: head/contrib/netbsd-tests/lib/libpthread/t_cond.c
==============================================================================
--- head/contrib/netbsd-tests/lib/libpthread/t_cond.c	Fri Mar  8 19:38:52 2019	(r344934)
+++ head/contrib/netbsd-tests/lib/libpthread/t_cond.c	Fri Mar  8 21:07:08 2019	(r344935)
@@ -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();

Modified: head/lib/libthr/thread/thr_cond.c
==============================================================================
--- head/lib/libthr/thread/thr_cond.c	Fri Mar  8 19:38:52 2019	(r344934)
+++ head/lib/libthr/thread/thr_cond.c	Fri Mar  8 21:07:08 2019	(r344935)
@@ -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);
 }