331169 - Obviate PR_Sleep before PR_GetConnectStatus on Windows

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Nelson Bolyard (seldom reads bugmail)]

After initiating a non-blocking PR_Connect, NSS's tstclnt.c source inserts 
a PR_Sleep call to sleep 1 millisecond between the call to PR_Poll and the 
subsequent call to PR_GetConnectStatus().  The comment for that feature says:

 * Must pause between PR_Poll and PR_GetConnectStatus,
 * Or else winsock gets mighty confused.

The CVS checkin log comment for that checkin said:

>Revision 1.17, Wed May 13 22:32:35 1998 UTC  by nelsonb
>
>Fix (work around) a couple of bugs in WINDOWS. (1) MUST call PR_Sleep
>between PR_Poll and PR_GetConnectStatus or winsock gets very confused.

Unfortunately, I don't recall if the problem was observed on Win NT or Win9x,
(or even Win16).  It seems to me that this PR_Sleep call belongs down inside
of PR_GetConnectStatus rather than in the application -- that is, if it is
still necesssary at all.  IIRC WTC suggested this fix to me way back then.

Wan-Teh do you remember this?  See bug 40778 for more background.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Besides tstclnt, libssl itself does the same thing in SSL_ImportFD and in ssl_Connect.

CVS blame history doesn't help in locating why this is done. Any tips? Is there a way to look at the version history of NSS before version 1.1 was checked into CVS?

Comment 2

[Wan-Teh Chang]

We should be able to remove that sleep of 1 millisecond.
I believe it is redundant with the Sleep(0) call in
PR_GetConnectStatus:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/pr/src/io/prsocket.c&rev=3.64&mark=294,324-328#294

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

According to Microsoft knowledgebase article Q165989 [1], this bug was fixed in Windows NT 4.0.

I think that we should only do the sleep in PR_GetConnectStatus() when the major windows version is 4.0 or lower, and remove the sleep calls in libssl.

[1] http://support.microsoft.com/kb/165989

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Remove obsolete Sleep(0) during socket connection on Windows

Wan-Teh, see the link in comment 3. This bug was fixed in Windows NT 4.0 SP4. I do not think we need to support pre-SP4 Windows NT 4.0 any more, so I did not make this conditional on a version check. I will file a bug in the NSS product for removing the libssl counterparts.

Comment 5

[Wan-Teh Chang]

Comment on attachment 745622 [details] [diff] [review]
Remove obsolete Sleep(0) during socket connection on Windows

Review of attachment 745622 [details] [diff] [review]:
-----------------------------------------------------------------

r=wtc.

Comment 6

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 745622 [details] [diff] [review]
Remove obsolete Sleep(0) during socket connection on Windows

http://hg.mozilla.org/projects/nspr/rev/fb7fff159c8c