Re: 3.0.1 patch for objc_mutex_trylock

[Ovidiu Predescu <ovidiu at cup dot hp dot com>]

The fix looks good to me. If we can fix the bug in 3.0.1, we should do
it, it's a rather big problem.

Greetings,
Ovidiu

On Tue, 17 Jul 2001 18:31:57 +0100 (BST), Nicola Pero <nicola@brainstorm.co.uk> wrote:

> It has just been reported by Stephen Brandon (thanks Stephen!) that
> objc_mutex_trylock was broken from 2.95 to 3.0.
> 
> Are we still in time to fix the bug in 3.0.1 ?  Hope so.
> 
> The problem is that in 2.95 the libobjc/thr-*.c code was used for the
> backend threading, while in 3.0 gcc/gthr-*.c is used.  For unknown
> reasons, the code in gcc/gthr-posix.c has been modified, and while the
> changes to __objc_mutex_trylook look trivial and innocent, they broke it.
> 
> the original thread backend function __objc_mutex_trylock in
> libobjc/thr-posix.c code is doing the following:
> 
> int
> __objc_mutex_trylock(objc_mutex_t mutex)
> {
>   if (pthread_mutex_trylock((pthread_mutex_t *)mutex->backend) == 0)
>     return 0;
>   else
>     return -1;
> }
> 
> the thread backend function __gthread_objc_mutex_trylock instead is doing:
> 
> static inline int
> __gthread_objc_mutex_trylock(objc_mutex_t mutex)
> {
>   if (__gthread_active_p ())
>     return pthread_mutex_trylock((pthread_mutex_t *)mutex->backend);
>   else
>     return 0;
> }
> 
> but this is quite different when an error occurs, because
> pthread_mutex_lock returns a positive number on error, while the calling
> code is expecting the backend thread function to return -1 on error, which
> is what the old __objc_mutex_trylock did, but not what the
> __gthread_objc_mutex_trylock is doing.
> 
> Ok for me to apply to 3.0.1 (and head) the following trivial patch which
> fixes the regression from 2.95.2 ?
> 
> 
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/ChangeLog,v
> retrieving revision 1.9240.2.703
> diff -u -r1.9240.2.703 ChangeLog
> --- ChangeLog   2001/07/17 12:51:17     1.9240.2.703
> +++ ChangeLog   2001/07/17 17:27:31
> @@ -1,3 +1,12 @@
> +Tue Jul 17 17:59:56 2001  Nicola Pero  <n.pero@mi.flashnet.it>
> +
> +       * gthr-posix.h (__gthread_objc_mutex_trylock): Fixed.  We can't
> +       blindly return the result of pthread_mutex_trylock because it
> +       might return a positive number on error, while we must return -1
> +       on error.
> +       (__gthread_objc_mutex_lock, __gthread_objc_mutex_unlock): Ditto.
> +       Reported by Stephen Brandon <stephen@brandonitconsulting.co.uk>.
> +
>  2001-07-17  H.J. Lu <hjl@gnu.org>
>             Rainer Orth <ro@TechFak.Uni-Bielefeld.DE>
>  
> 
> 
> Index: gthr-posix.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/gthr-posix.h,v
> retrieving revision 1.11.6.5
> diff -u -r1.11.6.5 gthr-posix.h
> --- gthr-posix.h        2001/06/28 12:53:47     1.11.6.5
> +++ gthr-posix.h        2001/07/17 17:20:32
> @@ -331,30 +331,39 @@
>  static inline int
>  __gthread_objc_mutex_lock(objc_mutex_t mutex)
>  {
> -  if (__gthread_active_p ())
> -    return pthread_mutex_lock((pthread_mutex_t *)mutex->backend);
> -  else
> -    return 0;
> +  if (__gthread_active_p () 
> +      && pthread_mutex_lock((pthread_mutex_t *)mutex->backend) != 0)
> +    {
> +      return -1;
> +    }
> +
> +  return 0;
>  }
>  
>  /* Try to grab a lock on a mutex. */
>  static inline int
>  __gthread_objc_mutex_trylock(objc_mutex_t mutex)
>  {
> -  if (__gthread_active_p ())
> -    return pthread_mutex_trylock((pthread_mutex_t *)mutex->backend);
> -  else
> -    return 0;
> +  if (__gthread_active_p () 
> +      && pthread_mutex_trylock((pthread_mutex_t *)mutex->backend) != 0)
> +    {
> +      return -1;
> +    }
> +
> +  return 0;
>  }
>  
>  /* Unlock the mutex */
>  static inline int
>  __gthread_objc_mutex_unlock(objc_mutex_t mutex)
>  {
> -  if (__gthread_active_p ())
> -    return pthread_mutex_unlock((pthread_mutex_t *)mutex->backend);
> -  else
> -    return 0;
> +  if (__gthread_active_p () 
> +      && pthread_mutex_unlock((pthread_mutex_t *)mutex->backend) != 0)
> +    {
> +      return -1;
> +    }
> +
> +  return 0;
>  }
>  
>  /* Backend condition mutex functions */
> 
> 
> 
> 
>