Open Bug 224277 Opened 21 years ago Updated 1 year ago

Error in _PR_WaitCondVar: invalid command sequence

Categories

(NSPR :: NSPR, defect)

x86
Windows 2000
defect

Tracking

(Not tracked)

People

(Reporter: synprak, Unassigned)

References

()

Details

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
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
Taking bug.
Assignee: dougt → wchang0222
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
Severity: normal → S3

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Assignee: wtc → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.