diff mbox

libstdc++/64847 add autoconf checks for pthread_rwlock_t

Message ID 20150312184956.GC9755@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely March 12, 2015, 6:49 p.m. UTC 
I assumed that Pthreads was enough to ensure pthread_rwlock_t but
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64847 shows that isn't
true for HPUX (seems it was optional prior to POSIX 1003.1-2001).

This adds an autoconf check to decide whether to use pthread_rwlock_t
or the fallback implementation in terms of std::condition_variable and
std::mutex.

This also includes some fixes from Torvald so that we loop and retry
if libc returns EAGAIN, to handle a difference in semantics between
POSIX and C++14.

And as an optimization I've made the _M_rwlock member use the
PTHREAD_RWLOCK_INITIALIZER macro if available.

Tested x86_64-linux, ppc64le-linux and x86_64-dragonfly.

I plan to commit this to trunk tomorrow.

Comments

Jonathan Wakely March 13, 2015, 1:46 p.m. UTC | #1 
On 12/03/15 18:49 +0000, Jonathan Wakely wrote:
>I assumed that Pthreads was enough to ensure pthread_rwlock_t but
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64847 shows that isn't
>true for HPUX (seems it was optional prior to POSIX 1003.1-2001).
>
>This adds an autoconf check to decide whether to use pthread_rwlock_t
>or the fallback implementation in terms of std::condition_variable and
>std::mutex.

Actually the autoconf changes aren't needed, as PR 64847 is already
fixed. Maybe it's better to have an explicit check for
pthread_rwlock_t anyway though ... I'll have a think.
diff mbox

Patch

commit 7212446ada7d741f6fe0fc9d9fca9d5b55322384
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Mar 12 17:29:42 2015 +0000

    2015-03-12  Jonathan Wakely  <jwakely@redhat.com>
    	    Torvald Riegel  <triegel@redhat.com>
    
    	PR libstdc++/64847
    	* acinclude.m4 (GLIBCXX_CHECK_GTHREADS): Check for pthread_rwlock_t.
    	* config.h.in: Regenerate.
    	* configure: Regenerate.
    	* include/std/shared_mutex: Check _GLIBCXX_USE_PTHREADS_RWLOCKS.
    	(shared_timed_mutex::_M_rwlock): Use PTHREAD_RWLOCK_INITIALIZER.
    	(shared_timed_mutex::lock_shared()): Retry on EAGAIN.
    	(shared_timed_mutex::try_lock_shared_until()): Retry on EAGAIN and
    	EDEADLK.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 1727140..86628c0 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -3563,6 +3563,13 @@  AC_DEFUN([GLIBCXX_CHECK_GTHREADS], [
   if test x"$ac_has_gthreads" = x"yes"; then
     AC_DEFINE(_GLIBCXX_HAS_GTHREADS, 1,
 	      [Define if gthreads library is available.])
+
+    # Also check for pthread_rwlock_t for std::shared_timed_mutex in C++14
+    AC_CHECK_TYPE([pthread_rwlock_t],
+            [AC_DEFINE([_GLIBCXX_USE_PTHREADS_RWLOCKS], 1,
+            [Define if POSIX read/write locks are available in <gthr.h>.])],
+            [],
+            [#include "gthr.h"])
   fi
 
   CXXFLAGS="$ac_save_CXXFLAGS"
diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex
index 5dcc295..61251b0 100644
--- a/libstdc++-v3/include/std/shared_mutex
+++ b/libstdc++-v3/include/std/shared_mutex
@@ -57,10 +57,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   /// shared_timed_mutex
   class shared_timed_mutex
   {
-#if defined(__GTHREADS_CXX0X)
+#ifdef _GLIBCXX_USE_PTHREADS_RWLOCKS
     typedef chrono::system_clock	__clock_t;
 
-    pthread_rwlock_t			_M_rwlock;
+#ifdef PTHREAD_RWLOCK_INITIALIZER
+    pthread_rwlock_t	_M_rwlock = PTHREAD_RWLOCK_INITIALIZER;
+
+  public:
+    shared_timed_mutex() = default;
+    ~shared_timed_mutex() = default;
+#else
+    pthread_rwlock_t	_M_rwlock;
 
   public:
     shared_timed_mutex()
@@ -82,6 +89,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // Errors not handled: EBUSY, EINVAL
       _GLIBCXX_DEBUG_ASSERT(__ret == 0);
     }
+#endif
 
     shared_timed_mutex(const shared_timed_mutex&) = delete;
     shared_timed_mutex& operator=(const shared_timed_mutex&) = delete;
@@ -165,12 +173,16 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     void
     lock_shared()
     {
-      int __ret = pthread_rwlock_rdlock(&_M_rwlock);
+      int __ret;
+      do
+	__ret = pthread_rwlock_rdlock(&_M_rwlock);
+      // We retry if we exceeded the maximum number of read locks supported by
+      // the POSIX implementation; this can result in busy-waiting, but this
+      // is okay based on the current specification of forward progress
+      // guarantees by the standard.
+      while (__ret == EAGAIN);
       if (__ret == EDEADLK)
 	__throw_system_error(int(errc::resource_deadlock_would_occur));
-      if (__ret == EAGAIN)
-	// Maximum number of read locks has been exceeded.
-	__throw_system_error(int(errc::device_or_resource_busy));
       // Errors not handled: EINVAL
       _GLIBCXX_DEBUG_ASSERT(__ret == 0);
     }
@@ -210,11 +222,19 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    static_cast<long>(__ns.count())
 	  };
 
-	int __ret = pthread_rwlock_timedrdlock(&_M_rwlock, &__ts);
+	int __ret;
+	do
+	  __ret = pthread_rwlock_timedrdlock(&_M_rwlock, &__ts);
 	// If the maximum number of read locks has been exceeded, or we would
-	// deadlock, we just fail to acquire the lock.  Unlike for lock(),
-	// we are not allowed to throw an exception.
-	if (__ret == ETIMEDOUT || __ret == EAGAIN || __ret == EDEADLK)
+	// deadlock, we just try to acquire the lock again (and will time out
+	// eventually).  Unlike for lock(), we are not allowed to throw an
+	// exception.  In cases where we would exceed the maximum number of
+	// read locks throughout the whole time until the timeout, we will
+	// fail to acquire the lock even if it would be logically free;
+	// however, this is allowed by the standard, and we made a "strong
+	// effort" (see C++14 30.4.1.4p26).
+	while (__ret == EAGAIN || __ret == EDEADLK);
+	if (__ret == ETIMEDOUT)
 	  return false;
 	// Errors not handled: EINVAL
 	_GLIBCXX_DEBUG_ASSERT(__ret == 0);
@@ -241,7 +261,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       unlock();
     }
 
-#else // defined(__GTHREADS_CXX0X)
+#else // !_GLIBCXX_USE_PTHREADS_RWLOCKS
 
 #if _GTHREAD_USE_MUTEX_TIMEDLOCK
     struct _Mutex : mutex, __timed_mutex_impl<_Mutex>
@@ -438,7 +458,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    _M_gate1.notify_one();
 	}
     }
-#endif // !defined(__GTHREADS_CXX0X)
+#endif // !_GLIBCXX_USE_PTHREADS_RWLOCKS
   };
 #endif // _GLIBCXX_HAS_GTHREADS