I am using the following FreeBSD version: 10.1-RELEASE-p6 FreeBSD 10.1-RELEASE-p6 #0 r279272: Wed Feb 25 12:27:40 CET 2015 built from the following svn branch: ^/releng/10.1 According to the man pages for the pthread_cond_destroy, it returns the following error codes: ERRORS The pthread_cond_destroy() function will fail if: [EINVAL] The value specified by cond is invalid. [EBUSY] The variable cond is locked by another thread. In the course of my testing I found that this function actually never returns EBUSY error code if the conditional variable is locked by another thread. Briefly looking at the pthread_cond_destroy implementation in /usr/src/lib/libthr/thread/thr_cond.c I don't see any checks for variable being locked by another thread: int _pthread_cond_destroy(pthread_cond_t *cond) { struct pthread_cond *cvp; int error = 0; if ((cvp = *cond) == THR_COND_INITIALIZER) error = 0; else if (cvp == THR_COND_DESTROYED) error = EINVAL; else { cvp = *cond; *cond = THR_COND_DESTROYED; /* * Free the memory allocated for the condition * variable structure: */ free(cvp); } return (error); } So it seems that one of the: documentation or the code is wrong. Some background about how it was tested. I am working on the Kea DHCP server implementation (kea.isc.org). One of our unit tests is exercising the scenario of trying to destroy the cond variable while it is locked by another thread. The expected behavior is that the pthread_cond_destroy fails with non-success status code, but it returns 0. However, this test uses C++ objects and infrastructure of our implementation and it hasn't been isolated as standalone test. If you find it useful to have such a standalone test which demonstrates the behavior, I can write one. For your convenience I am sending the locations of our source code where we run the test nested in our implementation: https://github.com/isc-projects/kea/blob/master/src/lib/util/threads/tests/condvar_unittest.cc (see the destroyWhileWait test). and the source code of the CondVar class under test: https://github.com/isc-projects/kea/blob/master/src/lib/util/threads/sync.cc Note that the destructor of the CondVar class calls the pthread_cond_destroy and then performs an assert that the status code is 0. For this test it should be non-zero and the process should die as expected by the test. But, it doesn't. Hope that helps.
Adding Konstantin to see what he thinks. First, the manpage should not say "locked" as you don't lock a condition variable (this appears to be copied from the pthread_mutex_destroy manpage and just not updated after it was copied). If it's going to fail with EBUSY it should be because there are threads blocked on the cv via pthread_cond_*wait(). Looking at the umtx bits, I think it would not be too hard to add a check that checked _has_user_waiters and __has_kern_waiters and failed with EBUSY if either were non-zero. The Open Group lists EBUSY as an optional check, but given that it should be cheap to do I would not be opposed to adding it (and also fixing the manpage to not say "locked").
(In reply to John Baldwin from comment #1) Such check is racy on both sides. It can report that condvar is busy while it is no longer such, and in reverse, it can delete a condvar which is started being used. Of course, if application allows such race, it is buggy. But my point is that threading library implementation cannot make this check non-racy without applicatin cooperation. That said, I would prefer not to add the check, at least because we cannot guarantee that EBUSY is returned always, and that what the error is returned, it actually happen. Might be, some wording in the man page explaining the details is due.
Don't the same races apply to pthread_mutex_destroy()?
(In reply to John Baldwin from comment #3) Hm, yes, this is very good point. I like your note also because it allows us to remove that code and silence asserts from gtk and other programs which destroy locked mutexes _and_ check error code from pthread_mutex_destroy.
I think warning about double destroys is a feature not a bug though. Yes, it is racy, but only for broken applications. A correctly written application will never get a spurious error from the EBUSY or is-destroyed checks for either mutexes or condvars. A buggy application might get them and might not, but in the case that it does get them it is helpful. I put gtk into the buggy application category btw. It's not just mutexes either, running any gtk/qt program on the console spews a ton of failed assertion warnings. Removing these checks would only make a small dent in the torrent of messages, but would remove checks that are helpful for other people who do check errors instead of ignoring them.
MARKED AS SPAM