FAForever / server

The servercode for the Forged Alliance Forever lobby
http://www.faforever.com
GNU General Public License v3.0
66 stars 61 forks source link

Sometimes games are rated three or more times #903

Closed Askaholic closed 2 years ago

Askaholic commented 2 years ago

There are some entries in the leaderboard_rating_journal that indicate that some games are being rated multiple times. The rating changes show that it is not just an issue of the same record being inserted multiple times, but actually the game appears to be rated multiple times.

Example for a single player: https://api.faforever.com/data/gamePlayerStats/33454629?include=ratingChanges

Example for the whole game: https://api.faforever.com/data/game/16909196?include=playerStats,playerStats.ratingChanges

You can see that every player in that game got 3 entries in the rating journal, so the game was rated 3 times in a row, and each time the players ratings after the previous change were used.

Askaholic commented 2 years ago

This is a race condition in the logic for triggering on_game_finish. The main entrypoint for this function getting called comes from GameConnection either when players disconnect: https://github.com/FAForever/server/blob/fbd13cac9ca4a5776da1a994ae184e6b4fe3cb82/server/games/game.py#L384 Or when the GameEnded command is sent: https://github.com/FAForever/server/blob/fbd13cac9ca4a5776da1a994ae184e6b4fe3cb82/server/gameconnection.py#L511

This function then checks if the game has already ended and then calls on_game_finish: https://github.com/FAForever/server/blob/fbd13cac9ca4a5776da1a994ae184e6b4fe3cb82/server/games/game.py#L393-L398

However, since on_game_finish only sets the game state to ENDED after it's done rating the game and writing the results to the database, it's possible for multiple tasks to kick off another call before the first one finishes. Probably the way to fix this is to add an async lock around the if statement above.