Error in _PR_WaitCondVar: invalid command sequence

ASSIGNED
Assigned to

Status

NSPR
NSPR
ASSIGNED
15 years ago
12 years ago

People

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

Tracking

x86
Windows 2000

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030624

Hi!
I CVS'd Moz 1.5 and (Boundschecker) found a problem in
nsprpub\pr\src\threads\combined\prucv.c
It complains that in

PRStatus _PR_WaitCondVar(PRThread *thread, PRCondVar *cvar, PRLock *lock,
PRIntervalTime timeout)

the sequence of unlocking (_PR_CVAR_UNLOCK(cvar); _PR_THREAD_UNLOCK(thread);) is
not correct since the allocation is also done in this order
(_PR_CVAR_LOCK(cvar); _PR_THREAD_LOCK(thread);)

imho the order of unlocking should be swapped.
I hope you got the point.

Afaik this 'problem' (is it really a problem??) also exists in v1.4

With kindest regards
  Philip

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Actual Results:  
Boundschecker complains

Expected Results:  
Swap the order of unlocking - mentioned in the description above

Comment 1

15 years ago
Confirmed that the 'unlocks' occur in the same order as the 'locks'. I would 
also expect them to be reversed, though whether this really makes any 
difference, I couldn't say.
Status: UNCONFIRMED → NEW
Component: Embedding: GRE Core → NSPR
Ever confirmed: true
Product: Browser → NSPR
Version: Trunk → 4.4
(Assignee)

Comment 2

15 years ago
Taking bug.
Assignee: dougt → wchang0222
(Assignee)

Comment 3

15 years ago
In that function, the order of unlocking doesn't need
to be the reverse of the order of locking.  What's
important is that while we are modifying data, both
locks are locked.  That is true regardless of the order
of unlocking.

I agree that it *looks nicer* to unlock in the reverse
order, so that the two critical sections are nested.
Could you try making that change and see if BoundsChecker
stops complaining?  If you could paste BoundsChecker's
complaint about the unlock order here, I'd appreciate it.
Status: NEW → ASSIGNED
QA Contact: nspr
You need to log in before you can comment on or make changes to this bug.