nsDNSService::DequeueLookup - must call PR_WaitCondVar within while loop

VERIFIED FIXED in mozilla0.9.2

Status

()

Core
Networking
P3
normal
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: gordon, Assigned: gordon)

Tracking

Trunk
mozilla0.9.2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

17 years ago
wtc spotted a bug in the dns service code that can result in the Run() method 
exit too early.  DequeueLookup() was not calling PR_WaitCondVar() from within a 
while loop.  

Wan-Teh, can you give me your r=, and Darin, your sr=?

Here's the patch:

Index: mozilla/netwerk/dns/src/nsDnsService.cpp
===================================================================
RCS file: /cvsroot/mozilla/netwerk/dns/src/nsDnsService.cpp,v
retrieving revision 1.84
diff -u -2 -r1.84 nsDnsService.cpp
--- nsDnsService.cpp	2001/06/08 04:54:07	1.84
+++ nsDnsService.cpp	2001/06/08 22:59:56
@@ -1406,5 +1406,5 @@
 #if defined(XP_UNIX)
     // Wait for notification of a queued request, unless  we're shutting down.
-    if (PR_CLIST_IS_EMPTY(&mPendingQ) && (mState != DNS_SHUTTING_DOWN)) {
+    while (PR_CLIST_IS_EMPTY(&mPendingQ) && (mState != DNS_SHUTTING_DOWN)) {
         PRStatus  status = PR_WaitCondVar(mDNSCondVar, PR_INTERVAL_NO_TIMEOUT);
         NS_ASSERTION(status == PR_SUCCESS, "PR_WaitCondVar failed.");
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.2

Comment 1

17 years ago
All PR_WaitCondVar calls must be in a while loop.

Is that the only PR_WaitCondVar call in that file?
(Assignee)

Comment 2

17 years ago
Created attachment 37744 [details] [diff] [review]
Protecting both calls to PR_WaitCondVar() with while loops.
(Assignee)

Comment 3

17 years ago
There was another PR_WaitCondVar that was unprotected (and has been for over a 
year).  I'll need to test this patch on Windows before I'll have confidence in 
it.

Comment 4

17 years ago
what about all the other countless places where we use PR_WaitCondVar, such as
the thread pool and others???

wtc: can you please enlighten me on this issue?  why is the while loop absolutely
necessary?  i thought that PR_WaitCondVar was supposed to block until
PR_NotifyCondVar is called?  is this not always the case?  thanks in advance!

Comment 5

17 years ago
Darin,

First, please understand that I am not nit-picking.  If I
had time I would examine all the uses of PR_WaitCondVar and
PR_Wait in Mozilla and make sure they are called inside a
while loop.

It is mandatory to call PR_WaitCondVar or PR_Wait inside a
while loop or an equivalent loop.  The key is that you must
test the boolean condition again after returning from PR_WaitCondVar
or PR_Wait.  Unfortunately a compiler cannot enforce this
as a syntactical requirement, so it is up to the programmer
to make sure this is done.

Another problem is to call PR_WaitCondVar or PR_Wait without
checking any boolean expression at all.  There are examples
of this sin in the same file (in XP_WIN or XP_WIN32 code).

Comment 6

17 years ago
r/sr=darin

Comment 7

17 years ago
I reviewed Gordon's patch.  It is good.  r=wtc@netscape.com.

Comment 8

17 years ago
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Blocks: 83989
(Assignee)

Comment 9

17 years ago
Patch checked in. Marking FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Updated

17 years ago
QA Contact: benc → bbaetz
code checked in. VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.