Resends to multiple clients may trash the send buffer

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Have either real or simulated latency and loss in the connection (for 
example 400ms simulated latency on the server)
2. Have multiple clients (3+) connect to the server
3. On the server, send reliable messages to the clients with high frequency

What is the expected output? 

Clients will eventually receive the messages, with data intact.

What do you see instead?

If the server receives acks from several clients on the same heartbeat, it will 
immediately queue resends on their NetConnections. However, the send buffer is 
shared by all connections on the server, 
so data to multiple clients potentially gets mixed in there. Then, when the 
NetConnection's Heartbeat() goes to queue further messages, there may be 
trashed/partial data in the sendbuffer, which will be prepended to the next 
packet sent to the client.

As a result, the client starts seeing "Malformed packet; stated payload 
length..." and "Packet parsing error: Got ack for message not yet sent?" 
messages, and not all messages are properly received.

Possible fixes would include either having a sendbuffer per connection, 
ensuring that resends always flush the sendbuffer by sending the packet 
(however this could be suboptimal, as the resends may be small), or moving 
resends to the NetConnection's Heartbeat().

What version of the product are you using? On what operating system?

SVN trunk version, Windows 7 64bit.

Original issue reported on code.google.com by loorni@gmail.com on 4 Feb 2012 at 9:26

[GoogleCodeExporter]

Are you talking about the queueing of acks on line 31 of 
NetReliableOrderedReceiver.cs for example? That line only adds an ack message 
to a queue; it's picked up in the heartbeat of the connection so there 
shouldn't be any thrashing there?

Original comment by lidg...@gmail.com on 6 Feb 2012 at 8:24

[GoogleCodeExporter]

I'm talking about this code flow:

NetPeer.Heartbeat() ->
sender.ReceivedLibraryMessage(tp, ptr, payloadByteLength) ->
chan.ReceiveAcknowledge(now, seqNr) ->

which, in NetReliableSenderChannel.cs line 250, may do a

m_connection.QueueSendMessage(rmsg, rnr);

which will cause corruption, if this code flow happens for several 
NetConnections (in NetPeer's packet receive loop) before proceeding to the 
connection-specific Heartbeats.

Original comment by loorni@gmail.com on 6 Feb 2012 at 11:11

[GoogleCodeExporter]

Alas! You are correct. Nasty bug; good find! I've checked in a fix in 284 where 
incoming acks are queued and handled in the NetConnection.Heartbeat method 
instead.

Original comment by lidg...@gmail.com on 6 Feb 2012 at 12:48

• Changed state: Started

[GoogleCodeExporter]

Have you checked if this solves your issue?

Original comment by lidg...@gmail.com on 1 Mar 2012 at 7:13

[GoogleCodeExporter]

Yes, it does solve it.

Original comment by loorni@gmail.com on 1 Mar 2012 at 8:59

[GoogleCodeExporter]

Original comment by lidg...@gmail.com on 22 Mar 2012 at 3:56

• Changed state: Fixed