[thread] ~mutex and BOOST_VERIFY
Hi, I saw that the destructor of ~mutex doesn't have a BOOST_VERIFY anymore on the return value of pthread_mutex_destroy. From SVN logs I can see it was removed in the commit 75882 to manage the EINTR due to some bugged POSIX implementation. I will reintroduce the BOOST_VERIFY like this: ~mutex() { int ret; do { ret = pthread_mutex_destroy(&m); } while (ret == EINTR); BOOST_VERIFY(!ret); } while we are at it consider the fact that for the same reason ~mutex needs to check for that EINTR return value the same should do timed_mutex. Regards Gaetano Mendola
On Sunday 26 May 2013 22:28:51 Gaetano Mendola wrote:
Hi, I saw that the destructor of ~mutex doesn't have a BOOST_VERIFY anymore on the return value of pthread_mutex_destroy. From SVN logs I can see it was removed in the commit 75882 to manage the EINTR due to some bugged POSIX implementation. I will reintroduce the BOOST_VERIFY like this:
~mutex() { int ret; do { ret = pthread_mutex_destroy(&m); } while (ret == EINTR); BOOST_VERIFY(!ret); }
while we are at it consider the fact that for the same reason ~mutex needs to check for that EINTR return value the same should do timed_mutex.
Are you sure pthread_mutex_destroy can return EINTR? My Linux man page as well as [1] explicitly states it can't. [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_init....
On 26/05/2013 22.39, Andrey Semashev wrote:
On Sunday 26 May 2013 22:28:51 Gaetano Mendola wrote:
Hi, I saw that the destructor of ~mutex doesn't have a BOOST_VERIFY anymore on the return value of pthread_mutex_destroy. From SVN logs I can see it was removed in the commit 75882 to manage the EINTR due to some bugged POSIX implementation. I will reintroduce the BOOST_VERIFY like this:
~mutex() { int ret; do { ret = pthread_mutex_destroy(&m); } while (ret == EINTR); BOOST_VERIFY(!ret); }
while we are at it consider the fact that for the same reason ~mutex needs to check for that EINTR return value the same should do timed_mutex.
Are you sure pthread_mutex_destroy can return EINTR? My Linux man page as well as [1] explicitly states it can't.
[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_init....
Well you know there is no only Linux out there and some of those other POSIX implementation are not 100% POSIX complaint, see ticket #6200, returning EINTR even if the are not supposed to do. Regards Gaetano Mendola
On Sunday 26 May 2013 22:59:55 Gaetano Mendola wrote:
On 26/05/2013 22.39, Andrey Semashev wrote:
On Sunday 26 May 2013 22:28:51 Gaetano Mendola wrote:
Hi, I saw that the destructor of ~mutex doesn't have a BOOST_VERIFY anymore on the return value of pthread_mutex_destroy. From SVN logs I can see it was removed in the commit 75882 to manage the EINTR due to some bugged POSIX implementation.
I will reintroduce the BOOST_VERIFY like this: ~mutex() {
int ret; do {
ret = pthread_mutex_destroy(&m);
} while (ret == EINTR); BOOST_VERIFY(!ret);
}
while we are at it consider the fact that for the same reason ~mutex needs to check for that EINTR return value the same should do timed_mutex.
Are you sure pthread_mutex_destroy can return EINTR? My Linux man page as well as [1] explicitly states it can't.
[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_ini t.html Well you know there is no only Linux out there and some of those other POSIX implementation are not 100% POSIX complaint, see ticket #6200, returning EINTR even if the are not supposed to do.
Then if it's a bug, it should be treated as such - through conditional compilation, so that it doesn't affect valid implementations. Maybe even as a distro-specific patch applied to Boost packages built for it. IMHO, of course. Do you have a particular affected platform in mind? Is there a way to detect it in compile time? PS: The check for EINTR is missing in other places than ~mutex() and ~timed_mutex(). I found at least ~lightweight_mutex(), ~recursive_mutex(), condition_variable(), condition_variable_any(), ~condition_variable_any(), as well as Boost.Interprocess components: ~posix_recursive_mutex() and ~posix_mutex() to be affected. It seems, you pretty much can't use Boost multithreading on such a buggy platform.
Andrey Semashev
On Sunday 26 May 2013 22:59:55 Gaetano Mendola wrote:
On 26/05/2013 22.39, Andrey Semashev wrote:
On Sunday 26 May 2013 22:28:51 Gaetano Mendola wrote:
Hi, I saw that the destructor of ~mutex doesn't have a BOOST_VERIFY anymore on the return value of pthread_mutex_destroy. From SVN logs I can see it was removed in the commit 75882 to manage the EINTR due to some bugged POSIX implementation.
I will reintroduce the BOOST_VERIFY like this: ~mutex() {
int ret; do {
ret = pthread_mutex_destroy(&m);
} while (ret == EINTR); BOOST_VERIFY(!ret);
}
while we are at it consider the fact that for the same reason ~mutex needs to check for that EINTR return value the same should do timed_mutex.
Are you sure pthread_mutex_destroy can return EINTR? My Linux man page as well as [1] explicitly states it can't.
[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_ini t.html Well you know there is no only Linux out there and some of those other POSIX implementation are not 100% POSIX complaint, see ticket #6200, returning EINTR even if the are not supposed to do.
Then if it's a bug, it should be treated as such - through conditional compilation, so that it doesn't affect valid implementations. Maybe even as a distro-specific patch applied to Boost packages built for it. IMHO, of course.
Do you have a particular affected platform in mind? Is there a way to detect it in compile time?
PS: The check for EINTR is missing in other places than ~mutex() and ~timed_mutex(). I found at least ~lightweight_mutex(), ~recursive_mutex(), condition_variable(), condition_variable_any(), ~condition_variable_any(), as well as Boost.Interprocess components: ~posix_recursive_mutex() and ~posix_mutex() to be affected. It seems, you pretty much can't use Boost multithreading on such a buggy platform.
I don't have a such bugged platform, I have just noticed the BOOST_VERIFY missing from mutex DTOR and looking at the svn log for it I found that the change was made for the ticket #6200. I do agree with you that in such platform boost thread can not be used as it is know. Regards Gaetano Mendola -- ----Android NewsGroup Reader---- http://www.piaohong.tk/newsgroup
Le 26/05/13 22:28, Gaetano Mendola a écrit :
Hi, I saw that the destructor of ~mutex doesn't have a BOOST_VERIFY anymore on the return value of pthread_mutex_destroy. From SVN logs I can see it was removed in the commit 75882 to manage the EINTR due to some bugged POSIX implementation. I will reintroduce the BOOST_VERIFY like this:
~mutex() { int ret; do { ret = pthread_mutex_destroy(&m); } while (ret == EINTR); BOOST_VERIFY(!ret); }
What do you want to verify?
while we are at it consider the fact that for the same reason ~mutex needs to check for that EINTR return value the same should do timed_mutex. yes, this could be done.
On Sunday 26 May 2013 23:45:48 Vicente J. Botet Escriba wrote:
Le 26/05/13 22:28, Gaetano Mendola a écrit :
Hi, I saw that the destructor of ~mutex doesn't have a BOOST_VERIFY anymore on the return value of pthread_mutex_destroy. From SVN logs I can see it was removed in the commit 75882 to manage the EINTR due to some bugged POSIX implementation.
I will reintroduce the BOOST_VERIFY like this: ~mutex() {
int ret; do {
ret = pthread_mutex_destroy(&m);
} while (ret == EINTR); BOOST_VERIFY(!ret);
}
What do you want to verify?
Checking the result may be useful for detecting incorrect operation sequence (e.g. when the mutex is destroyed with a blocked thread on it).
Gaetano Mendola wrote:
I will reintroduce the BOOST_VERIFY like this: ~mutex() {
int ret; do {
ret = pthread_mutex_destroy(&m);
} while (ret == EINTR); BOOST_VERIFY(!ret);
}
I'm not sure that this is "correct". You don't know that pthread_mutex_destroy can be retried if it returns EINTR. It might well fail the second time.
On 05/27/2013 01:39 PM, Peter Dimov wrote:
Gaetano Mendola wrote:
I will reintroduce the BOOST_VERIFY like this: ~mutex() {
int ret; do {
ret = pthread_mutex_destroy(&m);
} while (ret == EINTR); BOOST_VERIFY(!ret);
}
I'm not sure that this is "correct". You don't know that pthread_mutex_destroy can be retried if it returns EINTR. It might well fail the second time.
I'm not the author of that, I just notice that in my old boost version the BOOST_VERIFY was in place after our upgrade is not. Looking at the SVN log before the modification the DTOR was this: mutex::~mutex() { BOOST_VERIFY(!pthread_mutex_destroy(&m)); } since in the ticket #6200 someone found out that some POSIX implementations can return EINTR then the implementation of it was changed in what is the actual status. I have no idea what are the platforms with such behaviour so I can not even say if it's possible to retry the destruction or not. Regards Gaetano Mendola
Le 27/05/13 05:11, Andrey Semashev a écrit :
On Sunday 26 May 2013 23:45:48 Vicente J. Botet Escriba wrote:
Le 26/05/13 22:28, Gaetano Mendola a écrit :
Hi, I saw that the destructor of ~mutex doesn't have a BOOST_VERIFY anymore on the return value of pthread_mutex_destroy. From SVN logs I can see it was removed in the commit 75882 to manage the EINTR due to some bugged POSIX implementation.
I will reintroduce the BOOST_VERIFY like this: ~mutex() {
int ret; do {
ret = pthread_mutex_destroy(&m);
} while (ret == EINTR); BOOST_VERIFY(!ret);
} What do you want to verify? Checking the result may be useful for detecting incorrect operation sequence (e.g. when the mutex is destroyed with a blocked thread on it).
Thanks Andrey, I will take care of this, and add BOOST_VERIFY. Gaetano, could you create a ticket so that I don't forget it? Best, Vicente
On 28/05/2013 19.25, Vicente J. Botet Escriba wrote:
Le 27/05/13 05:11, Andrey Semashev a écrit :
On Sunday 26 May 2013 23:45:48 Vicente J. Botet Escriba wrote:
Le 26/05/13 22:28, Gaetano Mendola a écrit :
Hi, I saw that the destructor of ~mutex doesn't have a BOOST_VERIFY anymore on the return value of pthread_mutex_destroy. From SVN logs I can see it was removed in the commit 75882 to manage the EINTR due to some bugged POSIX implementation.
I will reintroduce the BOOST_VERIFY like this: ~mutex() { int ret; do { ret = pthread_mutex_destroy(&m); } while (ret == EINTR); BOOST_VERIFY(!ret); } What do you want to verify? Checking the result may be useful for detecting incorrect operation sequence (e.g. when the mutex is destroyed with a blocked thread on it).
Thanks Andrey,
I will take care of this, and add BOOST_VERIFY. Gaetano, could you create a ticket so that I don't forget it?
https://svn.boost.org/trac/boost/ticket/8626 Regards Gaetano Mendola
On 05/26/2013 11:45 PM, Vicente J. Botet Escriba wrote:
Le 26/05/13 22:28, Gaetano Mendola a écrit :
Hi, I saw that the destructor of ~mutex doesn't have a BOOST_VERIFY anymore on the return value of pthread_mutex_destroy. From SVN logs I can see it was removed in the commit 75882 to manage the EINTR due to some bugged POSIX implementation. I will reintroduce the BOOST_VERIFY like this:
~mutex() { int ret; do { ret = pthread_mutex_destroy(&m); } while (ret == EINTR); BOOST_VERIFY(!ret); }
What do you want to verify?
Same that the rest of thread library tests. This covers: 1) Mutex is unlocked 2) Use after destroy
while we are at it consider the fact that for the same reason ~mutex needs to check for that EINTR return value the same should do timed_mutex. yes, this could be done.
Doing a sanity check on this regard: $ rgrep pthread_mutex_destroy * pthread/mutex.hpp: ret = pthread_mutex_destroy(&m); pthread/mutex.hpp: BOOST_VERIFY(!pthread_mutex_destroy(&m)); pthread/mutex.hpp: BOOST_VERIFY(!pthread_mutex_destroy(&m)); pthread/condition_variable.hpp: BOOST_VERIFY(!pthread_mutex_destroy(&internal_mutex)); pthread/condition_variable.hpp: BOOST_VERIFY(!pthread_mutex_destroy(&internal_mutex)); pthread/condition_variable_fwd.hpp: BOOST_VERIFY(!pthread_mutex_destroy(&internal_mutex)); pthread/condition_variable_fwd.hpp: ret = pthread_mutex_destroy(&internal_mutex); pthread/recursive_mutex.hpp: BOOST_VERIFY(!pthread_mutex_destroy(&m)); pthread/recursive_mutex.hpp: BOOST_VERIFY(!pthread_mutex_destroy(&m)); pthread/recursive_mutex.hpp: BOOST_VERIFY(!pthread_mutex_destroy(&m)); pthread/recursive_mutex.hpp: BOOST_VERIFY(!pthread_mutex_destroy(&m)); Two "problems" arise 1) pthread/mutex.hpp misses the BOOST_VERIFY that was incorrectly removed in the commit for the ticket #6200, as you can see in the rest of the code the BOOST_VERIFY is used even in pthread/condition_variable_fwd.hpp is issued BOOST_ASSERT(!ret) due the fact that the mutex involved is an internal mutex. 2) As the ticket #6200 explains in some bogus POSIX implementation the pthread_mutex_destroy can return an EINTR and this case is only covered in pthread/mutex.hpp
participants (4)
-
Andrey Semashev -
Gaetano Mendola -
Peter Dimov -
Vicente J. Botet Escriba