TCP Games - Recap
I didn't feel like putting this up on my server, so here's it running locally. The order of operations were:
- Startup the server (top box)
- Connect with a client (bottom left), the server starts a Game
- Connect with a second client (bottom right), the server starts a second Game
- I fool around a bit until I get the correct answer in the bottom left client
- I then press Ctrl-C in the top box, shutting down the server, killing any running games and disconnecting (gracefully) any clients
As always, there are some problems with anything we do and a few things here we can improve on:
- I mentioned in the server programming part that there was a possible race condition with handling graceful client disconnects. To repeat myself, it was because the server may send a
Packetand then cleanup the network resources on its end before the client can process that
Packet(even though the ACK for that Packet should have been sent to the server before resources are cleaned up). We fixed it by sleeping the calling thread for 100ms. A possible solution might be that the client needs to respond by sending its own
- It's possible that the
_nextGamemight lock the main server thread when attempting to add players. Say if there was only one client in the lobby, but the game kept on rejecting adding them, we'd be in the loop at lines 84 through 95 forever. Any running games would still run, but nothing new would be able to start or connect.
- Just collecting
Taskobjects returned by
_handleNewConnection()(in the server) or
_handleIncomingPackets()(in the client) is bad. It's very possible that those Lists could grow to immense sizes. It would be better to create a structure that could house
Tasks until they've completed and then throw them into the garbage collector.
- The Games shouldn't really be handling client disconnects and
Packets as much as they do. In a much better designed application we would have a more complex "Player," object, handle all types of disconnects, and let the server dispatch
Packets to the Games. The games also notify the server when they are done, placing any current player back into the waiting lobby.
- There is a lot more we can parallelise and multithread in the server code. Just saying.
- /u/EntroperZero over on Reddit pointed out that our calls to
NetworkStreams return a
Taskthat tells us how many bytes were actually read (instead of how many were requested in the third parameter). Most of the time we will get the requested amount of bytes, but there is a chance that we could get less. We should be checking the requested count versus how many were actually read. You can read his original comment here.
I think this has been a good example of a multithreaded/async TCP server. I'm a little disappointed in myself that I got lazy and didn't implement Tic-Tac-Toe too, but I'll leave that as an exercise for you to do. I'd love to see it if you get it done (send me an email)! If you have any other suggestions for the review, drop me a line. And if you address any the problems I have listed above don't' hesitate to send me your solution.