Disconnect reason may fail to arrive when IP packets are lost

The server-side implementation of client disconnection interacts poorly with the TCP termination sequence, often causing TCP not to retransmit the Disconnect packet, and to send a RST signal instead. If the packet is lost in transit or the RST is reordered before it, the client will show a "Connection reset by peer" error instead of the intended disconnect text.

Additionally, (though I haven't tried it) this probably affects the new client transfer functionality as well.

Steps to reproduce

To reliably reproduce the problem, it is necessary to simulate an absurdly high rate of packet loss or reordering. On Linux the following should work (affects only the loopback interface; pick either or):

packet reordering:

# tc qdisc replace dev lo root netem delay 100ms reorder 50

packet loss (makes the connection very unreliable in general!):

# tc qdisc replace dev lo root netem loss 20

revert back to normal:

# tc qdisc replace dev lo root noqueue

Connect to a server running on the local machine, /kick yourself and observe the result. Try this a few times since the stars won't always align no matter what.

Mechanism

After sending a Disconnect packet, the server proceeds to call close() on the client socket in question. This corresponds to the (very confusingly named) "half-duplex" close sequence described in RFC 1122 section 4.2.2.13. Because the client sends data continuously, it is likely that some will arrive after the close. There may also already be some in the socket's receive buffer. Because the server has closed the socket and can no longer receive the data, per the aforementioned section, the TCP implementation "SHOULD [and at least on Linux, does] send a RST to show that data was lost." This causes the connection to enter the CLOSED state immediately, and discard all data being sent as well.

Despite this, the Disconnect packet usually arrives successfully: because the server enables TCP_NODELAY, the packet is sent out immediately, before the close, and in the absence of IP packet loss or reordering, arrives to the client before the RST. If those things do occur, however, the Disconnect packet is lost for good, and the client will (eventually) receive a RST or time out.

Suggested fix

After sending a Disconnect packet, the server should read from the socket, discarding the data, until the client closes the connection or some timeout is reached, and only then issue a close().

It may also make sense to shutdown(SHUT_WR) the server-side socket to immediately close its writing half, and indicate to the client that it should disconnect once it's done reading. This isn't strictly neceassary, since the Disconnect packet already causes the client to disconnect, and the timeout keeps the connection from hanging indefinitely even if that doesn't happen for whatever reason.