Skip to content

Commit 7b70ed8

Browse files
author
Dmitry Lenev
committed
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. --BZR-- revision-id: [email protected] property-branch-nick: mysql-next-4284-bg51105-2 property-file-info: ld7:file_id48:config.h.cmake-20091108233336-m9ewbh1c209yaw0h-37:message188:Check for presence of pthread_rwlockattr_setkind_np to be property-file-info: able to determine if system natively supports read-write property-file-info: locks for which we can specify if readers or writers should property-file-info: be preferred.4:path14:config.h.cmakeed7:file_id49:configure.cmake-20091108233336-m9ewbh1c209yaw0h-47:message188:Check for presence of pthread_rwlockattr_setkind_np to be property-file-info: able to determine if system natively supports read-write property-file-info: locks for which we can specify if readers or writers should property-file-info: be preferred.4:path15:configure.cmakeed7:file_id65:sp1f-configure.in-19700101030959-mgdpoxtnh2ewmvusvfpkreuhwvffkcjw7:message188:Check for presence of pthread_rwlockattr_setkind_np to be property-file-info: able to determine if system natively supports read-write property-file-info: locks for which we can specify if readers or writers should property-file-info: be preferred.4:path12:configure.ined7:file_id65:sp1f-my_pthread.h-19700101030959-z4yp3kljwx5fgmhlyvumtwxuw73xgrjn7:message180:Added support for portable read-write locks which prefer property-file-info: readers. property-file-info: To do so extended existing my_rw_lock_t implementation to property-file-info: support selection of whom to prefer depending on a flag.4:path20:include/my_pthread.hed7:file_id65:sp1f-thr_rwlock.c-19700101030959-aqyqud3lnjyia4mf5mse7u6zwnyajnye7:message201:Extended existing my_rw_lock_t implementation to support property-file-info: selection of whom to prefer depending on a flag. property-file-info: Added rw_pr_init() function implementing initialization of property-file-info: read-write locks preferring readers.4:path18:mysys/thr_rwlock.ced7:file_id40:mdl.cc-20080523121737-j62pi0m62eaw1hq6-17:message339:Use portable read-write locks which prefer readers instead of property-file-info: relying on that system implementation of read-write locks has property-file-info: this property (this was true for Linux/NPTL but was false, property-file-info: for example, for Windows and Solaris). property-file-info: Added comment explaining why preferring readers is important property-file-info: for MDL deadlock detector (thanks to Serg for example!).4:path10:sql/mdl.cced7:file_id39:mdl.h-20080523121748-o4y2wcq3maotb9do-17:message221:Use portable read-write locks which prefer readers instead of property-file-info: relying on that system implementation of read-write locks has property-file-info: this property (this was true for Linux/NPTL but was false, property-file-info: for example, for Windows and Solaris).4:path9:sql/mdl.hee testament3-sha1: 6f58cf2fdd491adfbc8a1f628f4dd149a6f4c37a
1 parent 6e4be6c commit 7b70ed8

7 files changed

Lines changed: 177 additions & 68 deletions

File tree

‎config.h.cmake‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@
205205
#cmakedefine HAVE_PTHREAD_KEY_DELETE 1
206206
#cmakedefine HAVE_PTHREAD_KILL 1
207207
#cmakedefine HAVE_PTHREAD_RWLOCK_RDLOCK 1
208+
#cmakedefine HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP 1
208209
#cmakedefine HAVE_PTHREAD_SETPRIO_NP 1
209210
#cmakedefine HAVE_PTHREAD_SETSCHEDPARAM 1
210211
#cmakedefine HAVE_PTHREAD_SIGMASK 1

‎configure.cmake‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,7 @@ CHECK_FUNCTION_EXISTS (pthread_condattr_setclock HAVE_PTHREAD_CONDATTR_SETCLOCK)
308308
CHECK_FUNCTION_EXISTS (pthread_init HAVE_PTHREAD_INIT)
309309
CHECK_FUNCTION_EXISTS (pthread_key_delete HAVE_PTHREAD_KEY_DELETE)
310310
CHECK_FUNCTION_EXISTS (pthread_rwlock_rdlock HAVE_PTHREAD_RWLOCK_RDLOCK)
311+
CHECK_FUNCTION_EXISTS (pthread_rwlockattr_setkind_np HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP)
311312
CHECK_FUNCTION_EXISTS (pthread_sigmask HAVE_PTHREAD_SIGMASK)
312313
CHECK_FUNCTION_EXISTS (pthread_threadmask HAVE_PTHREAD_THREADMASK)
313314
CHECK_FUNCTION_EXISTS (pthread_yield_np HAVE_PTHREAD_YIELD_NP)

‎configure.in‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2266,7 +2266,8 @@ AC_CHECK_FUNCS(alarm bcmp bfill bmove bsearch bzero \
22662266
locking longjmp lrand48 madvise mallinfo memcpy memmove \
22672267
mkstemp mlockall perror poll pread pthread_attr_create mmap mmap64 getpagesize \
22682268
pthread_attr_getstacksize pthread_attr_setstacksize pthread_condattr_create \
2269-
pthread_getsequence_np pthread_key_delete pthread_rwlock_rdlock pthread_sigmask \
2269+
pthread_getsequence_np pthread_key_delete pthread_rwlock_rdlock \
2270+
pthread_rwlockattr_setkind_np pthread_sigmask \
22702271
readlink realpath rename rint rwlock_init setupterm \
22712272
shmget shmat shmdt shmctl sigaction sigemptyset sigaddset \
22722273
sighold sigset sigthreadmask port_create sleep thr_yield \

‎include/my_pthread.h‎

Lines changed: 58 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -600,30 +600,76 @@ int my_pthread_fastmutex_lock(my_pthread_fastmutex_t *mp);
600600
#define my_rwlock_init(A,B) rwlock_init((A),USYNC_THREAD,0)
601601
#else
602602
/* Use our own version of read/write locks */
603-
typedef struct _my_rw_lock_t {
604-
pthread_mutex_t lock; /* lock for structure */
605-
pthread_cond_t readers; /* waiting readers */
606-
pthread_cond_t writers; /* waiting writers */
607-
int state; /* -1:writer,0:free,>0:readers */
608-
int waiters; /* number of waiting writers */
609-
} my_rw_lock_t;
610-
603+
#define NEED_MY_RW_LOCK 1
611604
#define rw_lock_t my_rw_lock_t
605+
#define my_rwlock_init(A,B) my_rw_init((A), 0)
612606
#define rw_rdlock(A) my_rw_rdlock((A))
613607
#define rw_wrlock(A) my_rw_wrlock((A))
614608
#define rw_tryrdlock(A) my_rw_tryrdlock((A))
615609
#define rw_trywrlock(A) my_rw_trywrlock((A))
616610
#define rw_unlock(A) my_rw_unlock((A))
617-
#define rwlock_destroy(A) my_rwlock_destroy((A))
611+
#define rwlock_destroy(A) my_rw_destroy((A))
612+
#endif /* USE_MUTEX_INSTEAD_OF_RW_LOCKS */
618613

619-
extern int my_rwlock_init(my_rw_lock_t *, void *);
620-
extern int my_rwlock_destroy(my_rw_lock_t *);
614+
615+
/*
616+
Portable read-write locks which prefer readers.
617+
618+
Required by some algorithms in order to provide correctness.
619+
*/
620+
621+
#if defined(HAVE_PTHREAD_RWLOCK_RDLOCK) && defined(HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP)
622+
/*
623+
On systems which have a way to specify that readers should
624+
be preferred through attribute mechanism (e.g. Linux) we use
625+
system implementation of read/write locks.
626+
*/
627+
#define rw_pr_lock_t pthread_rwlock_t
628+
extern int rw_pr_init(rw_pr_lock_t *);
629+
#define rw_pr_rdlock(A) pthread_rwlock_rdlock(A)
630+
#define rw_pr_wrlock(A) pthread_rwlock_wrlock(A)
631+
#define rw_pr_tryrdlock(A) pthread_rwlock_tryrdlock(A)
632+
#define rw_pr_trywrlock(A) pthread_rwlock_trywrlock(A)
633+
#define rw_pr_unlock(A) pthread_rwlock_unlock(A)
634+
#define rw_pr_destroy(A) pthread_rwlock_destroy(A)
635+
#else
636+
/* Otherwise we have to use our own implementation of read/write locks. */
637+
#define NEED_MY_RW_LOCK 1
638+
struct st_my_rw_lock_t;
639+
#define rw_pr_lock_t my_rw_lock_t
640+
extern int rw_pr_init(struct st_my_rw_lock_t *);
641+
#define rw_pr_rdlock(A) my_rw_rdlock((A))
642+
#define rw_pr_wrlock(A) my_rw_wrlock((A))
643+
#define rw_pr_tryrdlock(A) my_rw_tryrdlock((A))
644+
#define rw_pr_trywrlock(A) my_rw_trywrlock((A))
645+
#define rw_pr_unlock(A) my_rw_unlock((A))
646+
#define rw_pr_destroy(A) my_rw_destroy((A))
647+
#endif /* defined(HAVE_PTHREAD_RWLOCK_RDLOCK) && defined(HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP) */
648+
649+
650+
#ifdef NEED_MY_RW_LOCK
651+
/*
652+
On systems which don't support native read/write locks, or don't support
653+
read/write locks which prefer readers we have to use own implementation.
654+
*/
655+
typedef struct st_my_rw_lock_t {
656+
pthread_mutex_t lock; /* lock for structure */
657+
pthread_cond_t readers; /* waiting readers */
658+
pthread_cond_t writers; /* waiting writers */
659+
int state; /* -1:writer,0:free,>0:readers */
660+
int waiters; /* number of waiting writers */
661+
my_bool prefer_readers;
662+
} my_rw_lock_t;
663+
664+
extern int my_rw_init(my_rw_lock_t *, my_bool *);
665+
extern int my_rw_destroy(my_rw_lock_t *);
621666
extern int my_rw_rdlock(my_rw_lock_t *);
622667
extern int my_rw_wrlock(my_rw_lock_t *);
623668
extern int my_rw_unlock(my_rw_lock_t *);
624669
extern int my_rw_tryrdlock(my_rw_lock_t *);
625670
extern int my_rw_trywrlock(my_rw_lock_t *);
626-
#endif /* USE_MUTEX_INSTEAD_OF_RW_LOCKS */
671+
#endif /* NEED_MY_RW_LOCK */
672+
627673

628674
#define GETHOSTBYADDR_BUFF_SIZE 2048
629675

‎mysys/thr_rwlock.c‎

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
/* Synchronization - readers / writer thread locks */
1717

1818
#include "mysys_priv.h"
19-
#if defined(THREAD) && !defined(HAVE_PTHREAD_RWLOCK_RDLOCK) && !defined(HAVE_RWLOCK_INIT)
19+
#if defined(THREAD)
20+
#if defined(NEED_MY_RW_LOCK)
2021
#include <errno.h>
2122

2223
/*
@@ -58,7 +59,7 @@
5859
* Mountain View, California 94043
5960
*/
6061

61-
int my_rwlock_init(rw_lock_t *rwp, void *arg __attribute__((unused)))
62+
int my_rw_init(my_rw_lock_t *rwp, my_bool *prefer_readers_attr)
6263
{
6364
pthread_condattr_t cond_attr;
6465

@@ -70,12 +71,14 @@ int my_rwlock_init(rw_lock_t *rwp, void *arg __attribute__((unused)))
7071

7172
rwp->state = 0;
7273
rwp->waiters = 0;
74+
/* If attribute argument is NULL use default value - prefer writers. */
75+
rwp->prefer_readers= prefer_readers_attr ? *prefer_readers_attr : FALSE;
7376

7477
return(0);
7578
}
7679

7780

78-
int my_rwlock_destroy(rw_lock_t *rwp)
81+
int my_rw_destroy(my_rw_lock_t *rwp)
7982
{
8083
pthread_mutex_destroy( &rwp->lock );
8184
pthread_cond_destroy( &rwp->readers );
@@ -84,24 +87,26 @@ int my_rwlock_destroy(rw_lock_t *rwp)
8487
}
8588

8689

87-
int my_rw_rdlock(rw_lock_t *rwp)
90+
int my_rw_rdlock(my_rw_lock_t *rwp)
8891
{
8992
pthread_mutex_lock(&rwp->lock);
9093

9194
/* active or queued writers */
92-
while (( rwp->state < 0 ) || rwp->waiters)
95+
while (( rwp->state < 0 ) ||
96+
(rwp->waiters && ! rwp->prefer_readers))
9397
pthread_cond_wait( &rwp->readers, &rwp->lock);
9498

9599
rwp->state++;
96100
pthread_mutex_unlock(&rwp->lock);
97101
return(0);
98102
}
99103

100-
int my_rw_tryrdlock(rw_lock_t *rwp)
104+
int my_rw_tryrdlock(my_rw_lock_t *rwp)
101105
{
102106
int res;
103107
pthread_mutex_lock(&rwp->lock);
104-
if ((rwp->state < 0 ) || rwp->waiters)
108+
if ((rwp->state < 0 ) ||
109+
(rwp->waiters && ! rwp->prefer_readers))
105110
res= EBUSY; /* Can't get lock */
106111
else
107112
{
@@ -113,7 +118,7 @@ int my_rw_tryrdlock(rw_lock_t *rwp)
113118
}
114119

115120

116-
int my_rw_wrlock(rw_lock_t *rwp)
121+
int my_rw_wrlock(my_rw_lock_t *rwp)
117122
{
118123
pthread_mutex_lock(&rwp->lock);
119124
rwp->waiters++; /* another writer queued */
@@ -127,7 +132,7 @@ int my_rw_wrlock(rw_lock_t *rwp)
127132
}
128133

129134

130-
int my_rw_trywrlock(rw_lock_t *rwp)
135+
int my_rw_trywrlock(my_rw_lock_t *rwp)
131136
{
132137
int res;
133138
pthread_mutex_lock(&rwp->lock);
@@ -143,7 +148,7 @@ int my_rw_trywrlock(rw_lock_t *rwp)
143148
}
144149

145150

146-
int my_rw_unlock(rw_lock_t *rwp)
151+
int my_rw_unlock(my_rw_lock_t *rwp)
147152
{
148153
DBUG_PRINT("rw_unlock",
149154
("state: %d waiters: %d", rwp->state, rwp->waiters));
@@ -160,12 +165,39 @@ int my_rw_unlock(rw_lock_t *rwp)
160165
}
161166
else
162167
{
163-
if ( --rwp->state == 0 ) /* no more readers */
168+
if ( --rwp->state == 0 && /* no more readers */
169+
rwp->waiters)
164170
pthread_cond_signal( &rwp->writers );
165171
}
166172

167173
pthread_mutex_unlock( &rwp->lock );
168174
return(0);
169175
}
170176

171-
#endif
177+
178+
int rw_pr_init(struct st_my_rw_lock_t *rwlock)
179+
{
180+
my_bool prefer_readers_attr= TRUE;
181+
return my_rw_init(rwlock, &prefer_readers_attr);
182+
}
183+
184+
#else
185+
186+
/*
187+
We are on system which has native read/write locks which support
188+
preferring of readers.
189+
*/
190+
191+
int rw_pr_init(rw_pr_lock_t *rwlock)
192+
{
193+
pthread_rwlockattr_t rwlock_attr;
194+
195+
pthread_rwlockattr_init(&rwlock_attr);
196+
pthread_rwlockattr_setkind_np(&rwlock_attr, PTHREAD_RWLOCK_PREFER_READER_NP);
197+
pthread_rwlock_init(rwlock, NULL);
198+
pthread_rwlockattr_destroy(&rwlock_attr);
199+
return 0;
200+
}
201+
202+
#endif /* defined(NEED_MY_RW_LOCK) */
203+
#endif /* defined(THREAD) */

0 commit comments

Comments
 (0)