Fix for bug #51105 "MDL deadlock in rqg_mdl_stability test

on Windows".

On platforms where read-write lock implementation does not
prefer readers by default (Windows, Solaris) server might
have deadlocked while detecting MDL deadlock.

MDL deadlock detector relies on the fact that read-write
locks which are used in its implementation prefer readers
(see new comment for MDL_lock::m_rwlock for details).
So far MDL code assumed that default implementation of
read/write locks for the system has this property.
Indeed, this turned out ot be wrong, for example, for
Windows or Solaris. Thus MDL deadlock detector might have
deadlocked on these systems.

This fix simply adds portable implementation of read/write
lock which prefer readers and changes MDL code to use this
new type of synchronization primitive.

No test case is added as existing rqg_mdl_stability test can
serve as one.

config.h.cmake

@@ -205,6 +205,7 @@
 #cmakedefine HAVE_PTHREAD_KEY_DELETE 1
 #cmakedefine HAVE_PTHREAD_KILL 1
 #cmakedefine HAVE_PTHREAD_RWLOCK_RDLOCK 1
+#cmakedefine HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP 1
 #cmakedefine HAVE_PTHREAD_SETPRIO_NP 1
 #cmakedefine HAVE_PTHREAD_SETSCHEDPARAM 1
 #cmakedefine HAVE_PTHREAD_SIGMASK 1

configure.cmake

@@ -308,6 +308,7 @@ CHECK_FUNCTION_EXISTS (pthread_condattr_setclock HAVE_PTHREAD_CONDATTR_SETCLOCK)
 CHECK_FUNCTION_EXISTS (pthread_init HAVE_PTHREAD_INIT)
 CHECK_FUNCTION_EXISTS (pthread_key_delete HAVE_PTHREAD_KEY_DELETE)
 CHECK_FUNCTION_EXISTS (pthread_rwlock_rdlock HAVE_PTHREAD_RWLOCK_RDLOCK)
+CHECK_FUNCTION_EXISTS (pthread_rwlockattr_setkind_np HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP)
 CHECK_FUNCTION_EXISTS (pthread_sigmask HAVE_PTHREAD_SIGMASK)
 CHECK_FUNCTION_EXISTS (pthread_threadmask HAVE_PTHREAD_THREADMASK)
 CHECK_FUNCTION_EXISTS (pthread_yield_np HAVE_PTHREAD_YIELD_NP)

configure.in

@@ -2266,7 +2266,8 @@ AC_CHECK_FUNCS(alarm bcmp bfill bmove bsearch bzero \
   locking longjmp lrand48 madvise mallinfo memcpy memmove \
   mkstemp mlockall perror poll pread pthread_attr_create mmap mmap64 getpagesize \
   pthread_attr_getstacksize pthread_attr_setstacksize pthread_condattr_create \
-  pthread_getsequence_np pthread_key_delete pthread_rwlock_rdlock pthread_sigmask \
+  pthread_getsequence_np pthread_key_delete pthread_rwlock_rdlock \
+  pthread_rwlockattr_setkind_np pthread_sigmask \
   readlink realpath rename rint rwlock_init setupterm \
   shmget shmat shmdt shmctl sigaction sigemptyset sigaddset \
   sighold sigset sigthreadmask port_create sleep thr_yield \

include/my_pthread.h

@@ -600,30 +600,76 @@ int my_pthread_fastmutex_lock(my_pthread_fastmutex_t *mp);
 #define my_rwlock_init(A,B) rwlock_init((A),USYNC_THREAD,0)
 #else
 /* Use our own version of read/write locks */
-typedef struct _my_rw_lock_t {
-	pthread_mutex_t lock;		/* lock for structure		*/
-	pthread_cond_t	readers;	/* waiting readers		*/
-	pthread_cond_t	writers;	/* waiting writers		*/
-	int		state;		/* -1:writer,0:free,>0:readers	*/
-	int		waiters;	/* number of waiting writers	*/
-} my_rw_lock_t;
-
+#define NEED_MY_RW_LOCK 1
 #define rw_lock_t my_rw_lock_t
+#define my_rwlock_init(A,B) my_rw_init((A), 0)
 #define rw_rdlock(A) my_rw_rdlock((A))
 #define rw_wrlock(A) my_rw_wrlock((A))
 #define rw_tryrdlock(A) my_rw_tryrdlock((A))
 #define rw_trywrlock(A) my_rw_trywrlock((A))
 #define rw_unlock(A) my_rw_unlock((A))
-#define rwlock_destroy(A) my_rwlock_destroy((A))
+#define rwlock_destroy(A) my_rw_destroy((A))
+#endif /* USE_MUTEX_INSTEAD_OF_RW_LOCKS */
+
+
+/*
+  Portable read-write locks which prefer readers.
 
-extern int my_rwlock_init(my_rw_lock_t *, void *);
-extern int my_rwlock_destroy(my_rw_lock_t *);
+  Required by some algorithms in order to provide correctness.
+*/
+
+#if defined(HAVE_PTHREAD_RWLOCK_RDLOCK) && defined(HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP)
+/*
+  On systems which have a way to specify that readers should
+  be preferred through attribute mechanism (e.g. Linux) we use
+  system implementation of read/write locks.
+*/
+#define rw_pr_lock_t pthread_rwlock_t
+extern int rw_pr_init(rw_pr_lock_t *);
+#define rw_pr_rdlock(A) pthread_rwlock_rdlock(A)
+#define rw_pr_wrlock(A) pthread_rwlock_wrlock(A)
+#define rw_pr_tryrdlock(A) pthread_rwlock_tryrdlock(A)
+#define rw_pr_trywrlock(A) pthread_rwlock_trywrlock(A)
+#define rw_pr_unlock(A) pthread_rwlock_unlock(A)
+#define rw_pr_destroy(A) pthread_rwlock_destroy(A)
+#else
+/* Otherwise we have to use our own implementation of read/write locks. */
+#define NEED_MY_RW_LOCK 1
+struct st_my_rw_lock_t;
+#define rw_pr_lock_t my_rw_lock_t
+extern int rw_pr_init(struct st_my_rw_lock_t *);
+#define rw_pr_rdlock(A) my_rw_rdlock((A))
+#define rw_pr_wrlock(A) my_rw_wrlock((A))
+#define rw_pr_tryrdlock(A) my_rw_tryrdlock((A))
+#define rw_pr_trywrlock(A) my_rw_trywrlock((A))
+#define rw_pr_unlock(A) my_rw_unlock((A))
+#define rw_pr_destroy(A) my_rw_destroy((A))
+#endif /* defined(HAVE_PTHREAD_RWLOCK_RDLOCK) && defined(HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP) */
+
+
+#ifdef NEED_MY_RW_LOCK
+/*
+  On systems which don't support native read/write locks, or don't support
+  read/write locks which prefer readers we have to use own implementation.
+*/
+typedef struct st_my_rw_lock_t {
+	pthread_mutex_t lock;		/* lock for structure		*/
+	pthread_cond_t	readers;	/* waiting readers		*/
+	pthread_cond_t	writers;	/* waiting writers		*/
+	int		state;		/* -1:writer,0:free,>0:readers	*/
+	int             waiters;        /* number of waiting writers	*/
+	my_bool         prefer_readers;
+} my_rw_lock_t;
+
+extern int my_rw_init(my_rw_lock_t *, my_bool *);
+extern int my_rw_destroy(my_rw_lock_t *);
 extern int my_rw_rdlock(my_rw_lock_t *);
 extern int my_rw_wrlock(my_rw_lock_t *);
 extern int my_rw_unlock(my_rw_lock_t *);
 extern int my_rw_tryrdlock(my_rw_lock_t *);
 extern int my_rw_trywrlock(my_rw_lock_t *);
-#endif /* USE_MUTEX_INSTEAD_OF_RW_LOCKS */
+#endif /* NEED_MY_RW_LOCK */
+
 
 #define GETHOSTBYADDR_BUFF_SIZE 2048
 

mysys/thr_rwlock.c

@@ -16,7 +16,8 @@
 /* Synchronization - readers / writer thread locks */
 
 #include "mysys_priv.h"
-#if defined(THREAD) && !defined(HAVE_PTHREAD_RWLOCK_RDLOCK) && !defined(HAVE_RWLOCK_INIT)
+#if defined(THREAD)
+#if defined(NEED_MY_RW_LOCK)
 #include <errno.h>
 
 /*
@@ -58,7 +59,7 @@
 *  Mountain View, California  94043
 */
 
-int my_rwlock_init(rw_lock_t *rwp, void *arg __attribute__((unused)))
+int my_rw_init(my_rw_lock_t *rwp, my_bool *prefer_readers_attr)
 {
   pthread_condattr_t	cond_attr;
 
@@ -70,12 +71,14 @@ int my_rwlock_init(rw_lock_t *rwp, void *arg __attribute__((unused)))
 
   rwp->state	= 0;
   rwp->waiters	= 0;
+  /* If attribute argument is NULL use default value - prefer writers. */
+  rwp->prefer_readers= prefer_readers_attr ? *prefer_readers_attr : FALSE;
 
   return(0);
 }
 
 
-int my_rwlock_destroy(rw_lock_t *rwp)
+int my_rw_destroy(my_rw_lock_t *rwp)
 {
   pthread_mutex_destroy( &rwp->lock );
   pthread_cond_destroy( &rwp->readers );
@@ -84,12 +87,13 @@ int my_rwlock_destroy(rw_lock_t *rwp)
 }
 
 
-int my_rw_rdlock(rw_lock_t *rwp)
+int my_rw_rdlock(my_rw_lock_t *rwp)
 {
   pthread_mutex_lock(&rwp->lock);
 
   /* active or queued writers */
-  while (( rwp->state < 0 ) || rwp->waiters)
+  while (( rwp->state < 0 ) ||
+         (rwp->waiters && ! rwp->prefer_readers))
     pthread_cond_wait( &rwp->readers, &rwp->lock);
 
   rwp->state++;
@@ -97,11 +101,12 @@ int my_rw_rdlock(rw_lock_t *rwp)
   return(0);
 }
 
-int my_rw_tryrdlock(rw_lock_t *rwp)
+int my_rw_tryrdlock(my_rw_lock_t *rwp)
 {
   int res;
   pthread_mutex_lock(&rwp->lock);
-  if ((rwp->state < 0 ) || rwp->waiters)
+  if ((rwp->state < 0 ) ||
+      (rwp->waiters && ! rwp->prefer_readers))
     res= EBUSY;					/* Can't get lock */
   else
   {
@@ -113,7 +118,7 @@ int my_rw_tryrdlock(rw_lock_t *rwp)
 }
 
 
-int my_rw_wrlock(rw_lock_t *rwp)
+int my_rw_wrlock(my_rw_lock_t *rwp)
 {
   pthread_mutex_lock(&rwp->lock);
   rwp->waiters++;				/* another writer queued */
@@ -127,7 +132,7 @@ int my_rw_wrlock(rw_lock_t *rwp)
 }
 
 
-int my_rw_trywrlock(rw_lock_t *rwp)
+int my_rw_trywrlock(my_rw_lock_t *rwp)
 {
   int res;
   pthread_mutex_lock(&rwp->lock);
@@ -143,7 +148,7 @@ int my_rw_trywrlock(rw_lock_t *rwp)
 }
 
 
-int my_rw_unlock(rw_lock_t *rwp)
+int my_rw_unlock(my_rw_lock_t *rwp)
 {
   DBUG_PRINT("rw_unlock",
 	     ("state: %d waiters: %d", rwp->state, rwp->waiters));
@@ -160,7 +165,8 @@ int my_rw_unlock(rw_lock_t *rwp)
   }
   else
   {
-    if ( --rwp->state == 0 )	/* no more readers */
+    if ( --rwp->state == 0 &&   /* no more readers */
+        rwp->waiters)
       pthread_cond_signal( &rwp->writers );
   }
 
@@ -168,4 +174,30 @@ int my_rw_unlock(rw_lock_t *rwp)
   return(0);
 }
 
-#endif
+
+int rw_pr_init(struct st_my_rw_lock_t *rwlock)
+{
+  my_bool prefer_readers_attr= TRUE;
+  return my_rw_init(rwlock, &prefer_readers_attr);
+}
+
+#else
+
+/*
+  We are on system which has native read/write locks which support
+  preferring of readers.
+*/
+
+int rw_pr_init(rw_pr_lock_t *rwlock)
+{
+  pthread_rwlockattr_t rwlock_attr;
+
+  pthread_rwlockattr_init(&rwlock_attr);
+  pthread_rwlockattr_setkind_np(&rwlock_attr, PTHREAD_RWLOCK_PREFER_READER_NP);
+  pthread_rwlock_init(rwlock, NULL);
+  pthread_rwlockattr_destroy(&rwlock_attr);
+  return 0;
+}
+
+#endif /* defined(NEED_MY_RW_LOCK) */
+#endif /* defined(THREAD) */

sql/mdl.h

@@ -624,10 +624,11 @@ class MDL_context
   /**
     Read-write lock protecting m_waiting_for member.
 
-    TODO/FIXME: Replace with RW-lock which will prefer readers
-                on all platforms and not only on Linux.
+    @note The fact that this read-write lock prefers readers is
+          important as deadlock detector won't work correctly
+          otherwise. @sa Comment for MDL_lock::m_rwlock.
   */
-  rw_lock_t m_waiting_for_lock;
+  rw_pr_lock_t m_waiting_for_lock;
   MDL_ticket *m_waiting_for;
   uint m_deadlock_weight;
   /**
@@ -651,9 +652,9 @@ class MDL_context
 
   void will_wait_for(MDL_ticket *pending_ticket)
   {
-    rw_wrlock(&m_waiting_for_lock);
+    rw_pr_wrlock(&m_waiting_for_lock);
     m_waiting_for= pending_ticket;
-    rw_unlock(&m_waiting_for_lock);
+    rw_pr_unlock(&m_waiting_for_lock);
   }
 
   void set_deadlock_weight(uint weight)
@@ -669,9 +670,9 @@ class MDL_context
 
   void stop_waiting()
   {
-    rw_wrlock(&m_waiting_for_lock);
+    rw_pr_wrlock(&m_waiting_for_lock);
     m_waiting_for= NULL;
-    rw_unlock(&m_waiting_for_lock);
+    rw_pr_unlock(&m_waiting_for_lock);
   }
 
   void wait_reset()