Last Comment Bug 253080 - _PT_PTHREAD_MUTEX_IS_LOCKED macro broken, causes debug build to fail
: _PT_PTHREAD_MUTEX_IS_LOCKED macro broken, causes debug build to fail
Status: RESOLVED WONTFIX
:
Product: NSPR
Classification: Components
Component: NSPR (show other bugs)
: other
: x86 FreeBSD
: -- major (vote)
: ---
Assigned To: Wan-Teh Chang
:
Mentors:
Depends on:
Blocks: 264410 266981
  Show dependency treegraph
 
Reported: 2004-07-26 04:29 PDT by green
Modified: 2012-04-02 04:29 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
change check for error return of EBUSY to any error return (581 bytes, patch)
2004-07-26 04:31 PDT, green
no flags Details | Diff | Splinter Review

Description green 2004-07-26 04:30:00 PDT
User-Agent:       Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.7) Gecko/20040720 Firefox/0.9.1
Build Identifier: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.7) Gecko/20040720 Firefox/0.9.1

When WITH_DEBUG=1 is used, assertions that use the macro
_PT_PTHREAD_MUTEX_IS_LOCKED() fail consistently.  This is due to FreeBSD
returning the right POSIX error (EDEADLK) instead of EBUSY in the case where the
owner is trying to lock a locked, non-recursive mutex.

Reproducible: Always
Steps to Reproduce:
Comment 1 green 2004-07-26 04:31:18 PDT
Created attachment 154350 [details] [diff] [review]
change check for error return of EBUSY to any error return
Comment 2 Wan-Teh Chang 2004-07-26 20:59:46 PDT
Thanks for the bug report.  I remember dealing with this
problem for some other platform (AIX?) before.  I seem to
recall that in that case the vendor decided to change their
error code.

I remember the POSIX spec can be interpreted to say
either EBUSY or EDEADLK is correct.  It would be better if
we can define the macro to allow only EBUSY and EDEADLK,
rather than any error.
Comment 3 Vladimir Vukicevic [:vlad] [:vladv] 2004-10-11 23:18:28 PDT
(In reply to comment #2)
> I remember the POSIX spec can be interpreted to say
> either EBUSY or EDEADLK is correct.  It would be better if
> we can define the macro to allow only EBUSY and EDEADLK,
> rather than any error.

Does it matter, though?  Right now, if we get the "wrong" return value, code
assumes that the mutex is not locked, which seems far more incorrect than
assuming that the mutex is locked on EINVAL or EFAULT (which are far less
frequent cases that are bound to be cought elsewhere).  I'm not sure if calling
pthread_mutex_trylock twice to test for both EBUSY/EDEADLK in the macro is a
better alternative. 
Comment 4 Vladimir Vukicevic [:vlad] [:vladv] 2004-10-11 23:20:37 PDT
Actually, I take that back -- the code should probably just use the
PT_TRYLOCK_BUSY define (that's already different between _PR_DCETHREADS and the
rest of the world), and platforms which return EDEADLK can define it to that. 
_PT_PTHREAD_MUTEX_IS_LOCKED can then check against PT_TRYLOCK_BUSY.
Comment 5 Vladimir Vukicevic [:vlad] [:vladv] 2004-10-31 01:41:41 PDT
Comment on attachment 154350 [details] [diff] [review]
change check for error return of EBUSY to any error return

>-#define _PT_PTHREAD_MUTEX_IS_LOCKED(m)    (EBUSY == pthread_mutex_trylock(&(m)))
>+#define _PT_PTHREAD_MUTEX_IS_LOCKED(m)    (0 != pthread_mutex_trylock(&(m)))

Later on in _pth.h, there is a comment about pthread_mutex_trylock returning
different values on DCETHREADS vs. pthreads.  There's a define,
PT_TRYLOCK_BUSY, that's set to 0 on DCETHREADS and currently EBUSY on
everything else.  The _PT_PTHREAD_MUTEX_TRYLOCK macro should probably use
PT_TRYLOCK_BUSY, and appropriate sections added for various OS's that return
EDEADLK instead of EBUSY.

Then again, given that there's no way to do real error checking anyway, 0 might
just be easier.
Comment 6 Christophe Ravel 2007-05-22 14:37:55 PDT
The target milestone is already released. Resetting target milestone.
Comment 7 Phoenix 2012-02-27 05:41:13 PST
Is this still actual?

Note You need to log in before you can comment on or make changes to this bug.