smrealms / smr

Space Merchant Realms open-source game engine
http://www.smrealms.de
GNU Affero General Public License v3.0
25 stars 16 forks source link

Race condition in SmrSession #635

Open hemberger opened 5 years ago

hemberger commented 5 years ago

When players are logging in, they can sometimes trigger a duplicate entry in the active_session table. Analyzing the pathways for this to occur, it appears to only be possible when the same login is processed twice almost simultaneously.

MySQL Error MSG: Duplicate entry '...' for key 'PRIMARY'
Error Message: Exception: SQL query failed (INSERT INTO active_session (session_id, account_id, game_id, last_accessed, session_var) VALUES(...) in /smr/lib/Default/MySqlDatabase.class.inc:133
Stack trace:
#0 /smr/lib/Default/MySqlDatabase.class.inc(38): MySqlDatabase->error('SQL query faile...')
#1 /smr/lib/Default/SmrSession.class.inc(249): MySqlDatabase->query('INSERT INTO act...')
#2 /smr/htdocs/login_processing.php(220): SmrSession::update()
#3 {main}

This occurs on a fairly regular basis (maybe a few times a week to a few times a month), but the only way I have been able to reproduce it in development was by manually adding a sleep just before the INSERT and manually triggering it twice.

I have not been able to rapidly respond to players who encountered this issue, but one player a few hours later described their pre-error activity as (roughly) "logging in, going afk for a while, then trying to log in again from the same browser window".

This error could be avoided by using REPLACE instead of INSERT, but I feel like that only hides the underlying problem, which I do not fully understand yet.

hemberger commented 5 years ago

This simplified diagram of the logic illustrates how the error is impossible to trigger in a single-threaded case.


if (session_id exists in database) {
    generate = false
} else {
    generate = true
}

if (generate) {
    insert session_id into database   <-- this is where the error occurs
}