Hi Magnus!
Comments below.
Just my few cents,
Mats Kindahl
msvensson@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 5.0 repository of msvensson. When msvensson does a push these changes will
> be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2007-10-02 19:59:32+02:00, msvensson@stripped +3 -0
> 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.
> - it returns 0 if "mutex" was already taken => return EBUSY
> - it returns > 0 if "mutex" was aquired => return 0
>
> include/config-win.h@stripped, 2007-10-02 19:59:30+02:00, msvensson@stripped +3
> -0
> Make windows.h define TryEnterCriticalSection
>
> include/my_pthread.h@stripped, 2007-10-02 19:59:30+02:00, msvensson@stripped +1
> -2
> It's not possible to use WaitForSingleObject to wait
> on a CRITICAL_SECTION, instead use the TryEnterCriticalSection function.
> - it returns 0 if "mutex" was already taken => return EBUSY
> - it returns > 0 if "mutex" was aquired => return 0
>
> mysys/my_winthread.c@stripped, 2007-10-02 19:59:30+02:00, msvensson@stripped +0
> -24
> Remove 'win_pthread_mutext_trylock'
>
> diff -Nrup a/include/config-win.h b/include/config-win.h
> --- a/include/config-win.h 2007-03-28 23:00:25 +02:00
> +++ b/include/config-win.h 2007-10-02 19:59:30 +02:00
> @@ -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 -Nrup a/include/my_pthread.h b/include/my_pthread.h
> --- a/include/my_pthread.h 2007-10-01 15:14:53 +02:00
> +++ b/include/my_pthread.h 2007-10-02 19:59:30 +02:00
> @@ -116,7 +116,6 @@ 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);
> @@ -177,7 +176,7 @@ extern int pthread_mutex_destroy (pthrea
> #else
> #define pthread_mutex_init(A,B) (InitializeCriticalSection(A),0)
> #define pthread_mutex_lock(A) (EnterCriticalSection(A),0)
> -#define pthread_mutex_trylock(A) win_pthread_mutex_trylock((A))
> +#define pthread_mutex_trylock(A) (TryEnterCriticalSection(A) ? 0 : EBUSY)
>
If the current thread already own the critical section, the return value
shall be EBUSY. Since TryEnterCriticalSection returns a non-zero value
in this case, returning 0 is not correct according to POSIX.
EBUSY shall be returned if the thread owns the critical section or if
the critical section is already owned by another thread.
This might cause subtle bugs, so I suggest making sure that the return
code is correct in the event that the critical section is already owned
by the thread.
> #define pthread_mutex_unlock(A) LeaveCriticalSection(A)
> #define pthread_mutex_destroy(A) DeleteCriticalSection(A)
> #define my_pthread_setprio(A,B) SetThreadPriority(GetCurrentThread(), (B))
> diff -Nrup a/mysys/my_winthread.c b/mysys/my_winthread.c
> --- a/mysys/my_winthread.c 2007-10-01 15:11:12 +02:00
> +++ b/mysys/my_winthread.c 2007-10-02 19:59:30 +02:00
> @@ -40,30 +40,6 @@ 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
>
>
--
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com
