35408 - PR_NewTCPSocketPair needs to create a thread to do the connect

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P3)

Description

[Wan-Teh Chang]

PR_NewTCPSocketPair currently uses only one thread
to connect the two TCP sockets together.  It first
initiates a (nonblocking) connect, and then does an
accept.  It appears that this doesn't work with the
winsock of WinGate.  We may need to create a
temporary thread to do the connect.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed fix.

Comment 2

[Wan-Teh Chang]

Attachment: Fixed a typo in the previous attachment.

Comment 3

[Wan-Teh Chang]

There is a thread of discussion on this bug
in the mozilla.netlib newsgroup titled
"BIG WinGate Bug".

mkaply reported that PR_Connect failed with
the error codes (-5981, 10061). (10061 is
WSAECONNREFUSED.)  He also reported that my
proposed fix of creating a new thread to do
the connect didn't work.

Comment 4

[Wan-Teh Chang]

Reassigned the bug to larryh.

It turns out that the PR_Connect failure
has nothing to do with using the same thread
to do both the connect and accept.  The PR_Connect
failure is caused by WinGate.  It appears that
WinGate rejects all connections to the host's
IP address, even if the connections are from
the host itself.  However, Larry discovered that
if the listening socket is bound to the loopback
address (as opposed to a normal IP address of
the host), then WinGate allows connections from
the local host to that listening socket.

There is another problem on OS/2: the OS/2 TCP/IP
stack does not have the loopback address enabled
by default.  We will need to do a different
implementation of PR_NewPollableEvent on OS/2,
possibly using a pair of local sockets.

Comment 5

[ruslan]

Cool. I'm going to start checking if PR_NewPollableEvent fails and start using 
this 5ms poll in such case instead in the meantime.

Comment 6

[sewellpm]

Attachment: Analysis as reported to WinGate support

Comment 7

[larryh (gone)]

PR_NewTCPSocketPair() uses a pair of sockets on Win32. One socket, the listening 
socket, is bound to the host address of the local machine, the other socket is 
bound to the loopback address. When this is used with WinGate, WinGate appears 
to refuse the connection from the local machine's IP address connecting to the 
loopback address. I queried WinGate to see if this was the intended behavior? 
The have not responded to 2 queries.

As an experiment, I caused PR_NewTCPSocketPair() to use the loopback address on 
both sockets. This worked when used with the NSPR testcase pr/tests/pollable.c. 
I provided this patch to Mike Kaply (IBM, Austin) who reported the WinGate 
problem. He reports building NSPR, running it with his Mozilla and WinGate and 
it works for him.

I will change PR_NewTCPSocketPair() to use the loopback address for both ends of 
the socket pair for generic Win32. Other platforms will remain unchanged. ... 
What does this do to Windows NT? ... Comments?

See also: BugZilla 28403

Comment 8

[sewellpm]

WinNT will be just fine.  I've tested the method on this WinNt computer.  
Actually, INADDR_LOOPBACK is logically correct for all platforms.  With 
INADDR_ANY, the guy in the next office -- or someone on the other side of the 
world -- could probe your port at the wrong moment and cause your program to 
crash!

Be sure to test against HTTPS.  This same bug affects PSM (because PSM also 
calls PR_NewTCPSocketPair()).  PSM MAY also suffer from a similar oversight when 
setting up communication with Mozilla.

Comment 9

[larryh (gone)]

Checking in prsocket.c;
/cvsroot/mozilla/nsprpub/pr/src/io/prsocket.c,v  <--  prsocket.c
new revision: 3.30; previous revision: 3.29

Checked in repair to NSPR tip.

Comment 10

[Wan-Teh Chang]

Larry: I verified that your fix works on Mac.  The
pollable.c test passed.  Please go ahead and check
in your fix on both NSPRPUB_RELEASE_4_0_BRANCH and
NSPRPUB_CLIENT_BRANCH, without any ifdef's.

Ruslan: this shows that the pollable events work on
Mac too.  You might want to define USE_POLLABLE_EVENT
for Mac in nsSocketTransportService.h.

Comment 11

[larryh (gone)]

Checked in fix to NSPRPUB_RELEASE_4_0_BRANCH

Checking in prsocket.c;
/cvsroot/mozilla/nsprpub/pr/src/io/prsocket.c,v  <--  prsocket.c
new revision: 3.27.4.7; previous revision: 3.27.4.6

Comment 12

[larryh (gone)]

Checked into NSPRPUB_CLIENT_BRANCH

Checking in prsocket.c;
/cvsroot/mozilla/nsprpub/pr/src/io/prsocket.c,v  <--  prsocket.c
new revision: 3.28.2.1; previous revision: 3.28
done

Comment 13

[larryh (gone)]

marking this fixed.