search

EsotericSoftware / kryonet

TCP/UDP client/server library for Java, based on Kryo
BSD 3-Clause "New" or "Revised" License
1.81k stars 416 forks source link

To be sure that the response (method result) generated by the server is expected (yet) by a method call not timed-out. #71

Closed chechu closed 9 years ago

chechu commented 9 years ago

We are using this library in some projects, it's great. Thank you!

With this check we try to avoid that a method call gets as result the response really associated with a previous call that fisnished with a timeout.

The sequence that this code avoid: 1) A method call ends with timeout. The responseID associated is 1, and when the server finally returns the result this will be stored in the responseTable attribute. 2) 62 method calls come then without problems. 3) The next method call (with responseID = 1) will send the invokation via TCP and then it will wait the server response. As responseTable has a result for responseID=1 the last method call gets as result the result associated with the timed-out call :-(

I know that the solution that I propose is not perfect, but at least we can keep 63 parallel calls. I thought to use an UUID per method call, to identified it absolutly, but it would be slower than this solution.

What do you think? Do I forget something? Am I doing something wrong?

NathanSweet commented 9 years ago

Seems that the real problem is that 63 responses are not enough. I've added an extra byte, increasing it to 14 bits (16384 - 1 response IDs). We could use additional bits in the future if we wanted since this is probably higher than needed.

chechu commented 9 years ago

Excuse me, but I think that the problem does not disappear using 2 bytes. If a method call ends with a time out, after 16383 (now) calls the next method call will get a wrong result. This is the main problem, not the number of parallels calls.

NathanSweet commented 9 years ago

Ah sorry, misunderstood.

NathanSweet commented 9 years ago

Got it this time. I used your fix, thanks! Just changed it a bit to use an array rather than a set. I also added a test. The response ID bit packing stuff was contributed and needed a bit of refactoring anyway. Cheers!

chechu commented 9 years ago

Great! Yes, I prefer the array solution too. Good point. Thank you.