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);
 }
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to