72038 - FTP listings never stop loading on Mac

Status: VERIFIED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Simon Fraser [no longer active]]

The PR_Poll fixes on Mac broke FTP, such that directory listings (and maybe 
downloads) appear to never finish. The client never closes the data connection, 
with the result that the control connection waits for ever for the 'Transfer 
complete' notification.

The part of the PR_Poll fix that broke this was the change to GetState():

@@ -1676,6 +1681,8 @@
 {
     OTResult resultOT;
+	size_t   availableData;
     
-	*readReady = fd->secret->md.readReady;
+	resultOT = OTCountDataBytes((EndpointRef)fd->secret->md.osfd, &
availableData);   
+	*readReady = fd->secret->md.readReady && (availableData > 0);
 	*exceptReady = fd->secret->md.exceptReady;
 
This changed the behaviour of PR_Poll/PR_Read such that clients would no longer 
see a 0-byte read to indicate that all the data on a socket had been read (which 
is what FTP relied on).

Comment 1

[Simon Fraser [no longer active]]

Attachment: Fix for the bug, adds more logic to GetState()

Comment 2

[Simon Fraser [no longer active]]

The attached patch fixed the bug with FTP, and does not regress the original 
GetState fix (bug 70408).

Comment 3

[Doug Turner (:dougt)]

This must be fixed for 0.8.1

Comment 4

[Doug Turner (:dougt)]

gordon, can you review please?

Comment 5

[gordon]

Review it?  Simon and I wrote it.  Can you try the patch and verify it works for 
you?  I'll check it in this afternoon as soon as I get the necessary approvals.

Comment 6

[Scott Collins]

It would be better if you didn't use the old-style cast.  Since it's a type with
a simple name, you can use a construction-style cast.  Other than that, sr=scc.

Comment 7

[Christopher Blizzard (:blizzard)]

Assuming this gets properly tested and it's deemed low risk we're approving this
for 0.8.1.

Comment 8

[gordon]

Simon and I tested it, but I was hoping DougT was going to try some of his test 
cases on it as well.  Doug?

Comment 9

[gordon]

Fix checked in.

Comment 10

[Tom Everingham]

verified