From 2b9b71d0e3044572bc289701ae2a4055640a2bbb Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 1 Oct 2007 15:11:15 +0200 Subject: [PATCH 1/2] BUG#30992 (Wrong implementation of pthread_mutex_trylock()): Adding support for correct handling of pthread_mutex_trylock() on Win32 systems as well as when using the safe mutexes. include/my_pthread.h: Adding win_pthread_mutex_trylock() function as wrapper function for Windows implementation of pthread_mutex_trylock(). Adding flag to safe_mutex_lock() that is shall not abort if the mutex is already locked and instead return EBUSY since this is the POSIX behaviour. mysys/my_winthread.c: Added Win32 wrappper function to handle pthread_mutex_trylock(). mysys/thr_mutex.c: Added parameter 'try_lock' to safe_mutex_lock() to allow it to do double- duty as a pthread_mutex_trylock() in addition to working as pthread_mutex_lock(). The code needs to return EBUSY instead of throwing an error in the event that the mutex is re-locked (we do not have recursive mutexes), but it also requires some special handling to avoid race conditions when calling the real pthread_mutex_{lock,trylock}(). --- include/my_pthread.h | 9 ++++---- mysys/my_winthread.c | 25 ++++++++++++++++++++++ mysys/thr_mutex.c | 49 +++++++++++++++++++++++++++++++++++++------- 3 files changed, 72 insertions(+), 11 deletions(-) diff --git a/include/my_pthread.h b/include/my_pthread.h index d64b5348666..e28bb9df1f3 100644 --- a/include/my_pthread.h +++ b/include/my_pthread.h @@ -135,6 +135,7 @@ struct timespec { void win_pthread_init(void); int win_pthread_setspecific(void *A,void *B,uint length); +int win_pthread_mutex_trylock(pthread_mutex_t *mutex); int pthread_create(pthread_t *,pthread_attr_t *,pthread_handler,void *); int pthread_cond_init(pthread_cond_t *cond, const pthread_condattr_t *attr); int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex); @@ -195,7 +196,7 @@ extern int pthread_mutex_destroy (pthread_mutex_t *); #else #define pthread_mutex_init(A,B) (InitializeCriticalSection(A),0) #define pthread_mutex_lock(A) (EnterCriticalSection(A),0) -#define pthread_mutex_trylock(A) (WaitForSingleObject((A), 0) == WAIT_TIMEOUT) +#define pthread_mutex_trylock(A) win_pthread_mutex_trylock((A)) #define pthread_mutex_unlock(A) LeaveCriticalSection(A) #define pthread_mutex_destroy(A) DeleteCriticalSection(A) #define my_pthread_setprio(A,B) SetThreadPriority(GetCurrentThread(), (B)) @@ -593,7 +594,7 @@ typedef struct st_safe_mutex_info_t int safe_mutex_init(safe_mutex_t *mp, const pthread_mutexattr_t *attr, const char *file, uint line); -int safe_mutex_lock(safe_mutex_t *mp,const char *file, uint line); +int safe_mutex_lock(safe_mutex_t *mp, my_bool try_lock, const char *file, uint line); int safe_mutex_unlock(safe_mutex_t *mp,const char *file, uint line); int safe_mutex_destroy(safe_mutex_t *mp,const char *file, uint line); int safe_cond_wait(pthread_cond_t *cond, safe_mutex_t *mp,const char *file, @@ -616,12 +617,12 @@ void safe_mutex_end(FILE *file); #undef pthread_cond_timedwait #undef pthread_mutex_trylock #define pthread_mutex_init(A,B) safe_mutex_init((A),(B),__FILE__,__LINE__) -#define pthread_mutex_lock(A) safe_mutex_lock((A),__FILE__,__LINE__) +#define pthread_mutex_lock(A) safe_mutex_lock((A), FALSE, __FILE__, __LINE__) #define pthread_mutex_unlock(A) safe_mutex_unlock((A),__FILE__,__LINE__) #define pthread_mutex_destroy(A) safe_mutex_destroy((A),__FILE__,__LINE__) #define pthread_cond_wait(A,B) safe_cond_wait((A),(B),__FILE__,__LINE__) #define pthread_cond_timedwait(A,B,C) safe_cond_timedwait((A),(B),(C),__FILE__,__LINE__) -#define pthread_mutex_trylock(A) pthread_mutex_lock(A) +#define pthread_mutex_trylock(A) safe_mutex_lock((A), TRUE, __FILE__, __LINE__) #define pthread_mutex_t safe_mutex_t #define safe_mutex_assert_owner(mp) \ DBUG_ASSERT((mp)->count > 0 && \ diff --git a/mysys/my_winthread.c b/mysys/my_winthread.c index 27ccaef4f23..e9fface0521 100644 --- a/mysys/my_winthread.c +++ b/mysys/my_winthread.c @@ -40,6 +40,31 @@ void win_pthread_init(void) pthread_mutex_init(&THR_LOCK_thread,MY_MUTEX_INIT_FAST); } +/** + Adapter to @c pthread_mutex_trylock() + + @retval 0 Mutex was acquired + @retval EBUSY Mutex was already locked by a thread + @retval EINVAL Mutex could not be acquired due to other error + */ +int +win_pthread_mutex_trylock(pthread_mutex_t *mutex) +{ + switch (WaitForSingleObject(mutex, 0)) { + case WAIT_TIMEOUT: + return EBUSY; + + default: + case WAIT_FAILURE: + return EINVAL; + + case WAIT_OBJECT_0: + case WAIT_ABANDONED: /* The mutex was acquired because it was + * abandoned */ + return 0; + } +} + /* ** We have tried to use '_beginthreadex' instead of '_beginthread' here ** but in this case the program leaks about 512 characters for each diff --git a/mysys/thr_mutex.c b/mysys/thr_mutex.c index 425e5fce459..53ee907e0a3 100644 --- a/mysys/thr_mutex.c +++ b/mysys/thr_mutex.c @@ -91,7 +91,7 @@ int safe_mutex_init(safe_mutex_t *mp, } -int safe_mutex_lock(safe_mutex_t *mp,const char *file, uint line) +int safe_mutex_lock(safe_mutex_t *mp, my_bool try_lock, const char *file, uint line) { int error; if (!mp->file) @@ -104,15 +104,50 @@ int safe_mutex_lock(safe_mutex_t *mp,const char *file, uint line) } pthread_mutex_lock(&mp->global); - if (mp->count > 0 && pthread_equal(pthread_self(),mp->thread)) + if (mp->count > 0) { - fprintf(stderr,"safe_mutex: Trying to lock mutex at %s, line %d, when the mutex was already locked at %s, line %d in thread %s\n", - file,line,mp->file, mp->line, my_thread_name()); - fflush(stderr); - abort(); + if (try_lock) + { + pthread_mutex_unlock(&mp->global); + return EBUSY; + } + else if (pthread_equal(pthread_self(),mp->thread)) + { + fprintf(stderr, + "safe_mutex: Trying to lock mutex at %s, line %d, when the" + " mutex was already locked at %s, line %d in thread %s\n", + file,line,mp->file, mp->line, my_thread_name()); + fflush(stderr); + abort(); + } } pthread_mutex_unlock(&mp->global); - error=pthread_mutex_lock(&mp->mutex); + + /* + If we are imitating trylock(), we need to take special + precautions. + + - We cannot use pthread_mutex_lock() only since another thread can + overtake this thread and take the lock before this thread + causing pthread_mutex_trylock() to hang. In this case, we should + just return EBUSY. Hence, we use pthread_mutex_trylock() to be + able to return immediately. + + - We cannot just use trylock() and continue execution below, since + this would generate an error and abort execution if the thread + was overtaken and trylock() returned EBUSY . In this case, we + instead just return EBUSY, since this is the expected behaviour + of trylock(). + */ + if (try_lock) + { + error= pthread_mutex_trylock(&mp->mutex); + if (error == EBUSY) + return error; + } + else + error= pthread_mutex_lock(&mp->mutex); + if (error || (error=pthread_mutex_lock(&mp->global))) { fprintf(stderr,"Got error %d when trying to lock mutex at %s, line %d\n", From 662fb20f1e03a5d4146a67b5e73c828f5ab476e4 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 3 Oct 2007 21:38:32 +0200 Subject: [PATCH 2/2] Bug#30992 Wrong implementation of pthread_mutex_trylock() It's not possible to use WaitForSingleObject to wait on a CRITICAL_SECTION, instead use the TryEnterCriticalSection function. - if "mutex" was already taken => return EBUSY - if "mutex" was aquired => return 0 include/config-win.h: Make windows.h define TryEnterCriticalSection mysys/my_winthread.c: Use the TryEnterCriticalSection function to implement pthread_mutex_trylock Prevent recursive behaviour by looking at the RecursionCount variable in the CRITICAL_SECTION struct and return EBUSY from function if the mutex was already locked once. --- include/config-win.h | 3 +++ mysys/my_winthread.c | 22 ++++++++++------------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/include/config-win.h b/include/config-win.h index 279be7aa5e4..f0804a368bc 100644 --- a/include/config-win.h +++ b/include/config-win.h @@ -19,6 +19,9 @@ /* We have to do this define before including windows.h to get the AWE API functions */ #define _WIN32_WINNT 0x0500 +#else +/* Get NT 4.0 functions */ +#define _WIN32_WINNT 0x0400 #endif #if defined(_MSC_VER) && _MSC_VER >= 1400 diff --git a/mysys/my_winthread.c b/mysys/my_winthread.c index e9fface0521..e94369bec32 100644 --- a/mysys/my_winthread.c +++ b/mysys/my_winthread.c @@ -40,31 +40,29 @@ void win_pthread_init(void) pthread_mutex_init(&THR_LOCK_thread,MY_MUTEX_INIT_FAST); } + /** Adapter to @c pthread_mutex_trylock() @retval 0 Mutex was acquired @retval EBUSY Mutex was already locked by a thread - @retval EINVAL Mutex could not be acquired due to other error */ int win_pthread_mutex_trylock(pthread_mutex_t *mutex) { - switch (WaitForSingleObject(mutex, 0)) { - case WAIT_TIMEOUT: - return EBUSY; - - default: - case WAIT_FAILURE: - return EINVAL; - - case WAIT_OBJECT_0: - case WAIT_ABANDONED: /* The mutex was acquired because it was - * abandoned */ + if (TryEnterCriticalSection(mutex)) + { + /* Don't allow recursive lock */ + if (mutex->RecursionCount > 1){ + LeaveCriticalSection(mutex); + return EBUSY; + } return 0; } + return EBUSY; } + /* ** We have tried to use '_beginthreadex' instead of '_beginthread' here ** but in this case the program leaks about 512 characters for each