ClientPrefs cookie creation racing clients

[InvexByte]

For context and information on the issue please read: https://forums.alliedmods.net/showthread.php?t=296658

---

Summary

• This is not a new bug. I've had this issue for at least 10+ months.
• Plugin (multi1v1) uses clientprefs to store cookies for various things (On git here: https://github.com/splewis/csgo-multi-1v1)
• My server runs a GOTV bot 24/7 to record demos.
• If server is started without a GOTV client (i.e. without tv_enable 1 in server.cfg), plugin works normally
• If server is started with a GOTV client, after some time the clientprefs database will corrupt. Once database is in a corrupt state, invalid rows are inserted into the clientprefs database (sm_cookie_cache table). The invalid rows have same player field (auth id) and same name field but an invalid cookie_id (that does not exist in the sm_cookies table). See: https://i.imgur.com/VQ1v6DF.png https://i.imgur.com/p9BahlN.png
• If you then start server with an corrupt/invalid clientprefs database but without a GOTV client, the cookies are set properly again and persist on map change. Start with the GOTV client and it is bugged.
• If you start the server without the GOTV bot but then manually add the GOTV bot in after joining, cookies are set properly and persist on map changes.
• If you start the server with the GOTV bot and kick it from the game, cookies are broken and do not work even after the GOTV client is kicked/map changes occur etc.
• Regular fake clients (CSGO bots) do not cause the issue

---

Replicating the Bug

• Unfortunately I have yet to discover how to replicate the bug when started with a fresh clientprefs database.
• If I start a server with the GOTV client, the multi1v1 plugin and 2 real players. I can map change and save settings for a good long time (30 minutes) without any issues.
• However, if I delete the clientprefs server on my main server (which regularly gets 20-24 players), within 1-6 hours, the database will almost certainly be in an corrupt/invalid state.

---

This may or may not be a SM bug. May just be a plugin bug. However, it does seem that SM allows the clientprefs database to reach an invalid state (with entries in sm_cookies_cache containing duplicate rows with an invalid non-existent cookie_id).

Comments by psychonic and asherkin in IRC:

<psychonic> Byte, file an Issue
<psychonic> Blind guess at the bug is issue with detecting hltv in core after map change combined with validation bug in cp
<psychonic> Haven’t been able to dig into it yet. Could have been triggered by a change in the engine
<asherkin> I don't see how a client could have anything to do with the cookie id getting corrupted
<asherkin> I think it is more because of timing when it is joining
<asherkin> there are definitely races with cookie loading, the code is a mess

Any help/ideas to test are appreciated.

[KyleSanderson]

Whoa, yeah; RegClientCookie is typically only called at plugin startup, with clients joining afterwards. GoTV already being present, and calling this afterwards is as just asking for trouble. I'm sure you've noticed that multi1v1_autospec and multi1v1_hidestats are fine, because they're not hidden away.

At one point, this seemed a little more familiar to everything else. https://github.com/splewis/csgo-multi-1v1/blob/2710defb25f7f628cb6c950379325b54ed55d120/scripting/multi1v1/generic.sp#L96 cookies were already generated, and created via https://github.com/splewis/csgo-multi-1v1/blob/2710defb25f7f628cb6c950379325b54ed55d120/scripting/multi1v1.sp#L160

Was splewis wrong for doing this? no. Reverting back to that known pattern that was there would fix this, or if he calls CloseHandle(FindNamedCookie("cookies")) in OnPluginStart that would almost certainly paper over this.