RE: Patch to mutex.c

[Neale dot Ferguson at SoftwareAG-USA dot com]

Title: RE: Patch to mutex.c

Thanks for the quick reponse.

re: alt_unlock()
----------------

for PTHREAD_MUTEX_ERRORCHECK_NP types, pthread_mutex_lock will
 - call __pthread_alt_lock() which will:
 - check that lock->__status == 0, if so it will set newstatus to 1
                                   else set newstatus to &wait_node
 - compare_and_swap making sure __status still = oldstatus and if so swap in newstatus to __status (i.e. 1 or &wait_node)

for PTHREAD_MUTEX_ERRORCHECK_NP types, __pthread_mutex_unlock will:
 - check that the lsb of __status is on (and owner is the current thread)
 - if so, it will call alt_unlock()
 - However, if __status actually == &wait_node then it will not call alt_unlock() and just return EPERM (unless address of wait_node is on an oddbyte boundary)

so: should alt_lock() set newstatus = &wait_node ¦ 1L
  
re: recursive_np
----------------
My concern here is that if a program does call pthread_mutex_unlock() "too many times" then _pthread_unlock will:

- fail the while((oldstatus  = lock->__status) == 1) test, with oldstatus = 0x00.
- ptr will be set to &lock->__status (ok) and thr = oldstatus & ~1L (in this instance 0x00).
- The while(thr != 0) will fail and the next instruction executed will be
- thr->p_nextlock = NULL which means writing into location 0x00. On our system that means crash! 

-----Original Message-----
From: Kaz Kylheku
To: Neale.Ferguson@SoftwareAG-USA.com
Cc: libc-alpha@sources.redhat.com
Sent: 9/20/00 12:41 PM
Subject: RE: Patch to mutex.c

On Wed, 20 Sep 2000 Neale.Ferguson@SoftwareAG-USA.com wrote:

> Does that mean my fix is not usable and my analysis flawed (not
atypical I'm
> afraid ;-))? If so, where did I go wrong?

The patch breaks correct behavior.  I think where you went wrong was by
wrongly
applying knowledge of the fastlock internals to the alternate lock which
is
different.

> I've also noted in pthread_mutex_unlock() for mutex kinds of
> PTHREAD_MUTEX_RECURSIVE_NP that if a mutex has been unlocked such that
the
> count is 0 then _pthread_unlock() is called. If a routine were to call
> pthread_mutex_unlock() again with the same mutex then
_pthread_unlock() is
> called again which causes grief. Shouldn't there be a test like...
>
> if (mutex->__m_owner != NULL) {
>   mutex->__m_owner = NULL;
>   __pthread_unlock(&mutex->__m_lock);
>   return 0;
> }
> else
>   return EPERM;

No. According to the applicable standards, this error check is
permitted, but
not required.  The behavior is simply undefined.  The rationale is that
the
check is a waste of time in provably correct programs, which can just
ignore
the return value of these operations because it is always zero. Except
in the
case of PTHREAD_MUTEX_ERRORCHECK, implementors have a choice whether or
not to
implement the checks.

Unfortunately, there is currently no
PTHREAD_MUTEX_ERRORCHECK_RECURSIVE_NP for
people who are looking for recursive behavior as well as error checking.
It may be wortwhile to create this.