The default bug view has changed. See this FAQ.

_PT_PTHREAD_MUTEX_IS_LOCKED macro broken, causes debug build to fail

RESOLVED WONTFIX

Status

NSPR
NSPR
--
major
RESOLVED WONTFIX
13 years ago
5 years ago

People

(Reporter: green, Assigned: Wan-Teh Chang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

13 years ago
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:
(Reporter)

Comment 1

13 years ago
Created attachment 154350 [details] [diff] [review]
change check for error return of EBUSY to any error return
(Assignee)

Comment 2

13 years ago
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.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(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. 
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.
Blocks: 264410
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.
(Assignee)

Updated

13 years ago
Target Milestone: --- → 4.6
(Assignee)

Updated

13 years ago
Blocks: 266981
QA Contact: wtchang → nspr

Comment 6

10 years ago
The target milestone is already released. Resetting target milestone.
Target Milestone: 4.6 → ---

Comment 7

5 years ago
Is this still actual?
Status: ASSIGNED → NEW
Whiteboard: closeme WONT 2012-04-01

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
Whiteboard: closeme WONT 2012-04-01
You need to log in before you can comment on or make changes to this bug.