Page MenuHomeWildfire Games

Replace NetTurnManager ENSURE spam with LOGERROR + disconnect

Authored by elexis on Jan 25 2018, 6:33 PM.



As reported in #3643, a client repeatedly joining and cancelling the join can trigger a debug breakpoint in the netturnmanager.
The debug breakpoint doesn't help the players at all but can even prevent them from playing and cancel the match.
Since the breakpoint can be triggered by trolls, it would be better to defuse it, even if we don't know the root cause yet.

Test Plan

Apply this patch to reproduce the breakpoint by just joining a game.

Diff Detail

rP 0 A.D. Public Repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Jan 25 2018, 6:33 PM
elexis edited the test plan for this revision. (Show Details)Jan 25 2018, 6:36 PM
echotangoecho added inline comments.Jan 26 2018, 7:22 PM
41 ↗(On Diff #5490)

Why not take CNetServerSession by reference here?

The change would be unneeded if it became impossible for clients to trigger that ENSURE.
But someone changing the code can still trigger this manually at any time after the connect.
So as far as I see the ENSURE must go either way.
This doesn't mean that the reported bug is fixed, but that is not the purpose of this patch (hence it will not fix the ticket).

41 ↗(On Diff #5490)

Because it's already defined as a pointer and it would be consistent with the NetServerWorker functions. But I can change it. Anything else?

echotangoecho accepted this revision.Jan 26 2018, 8:08 PM

If you change the pointers -> reference, it can be committed.

This revision is now accepted and ready to land.Jan 26 2018, 8:08 PM

Thanks for the review echotangoecho.

If someone can figure out why the legit bug occurs to begin with, that would be appreciated, but last time I checked (2 years ago) I couldn't find it and some more time will have to pass until I get submerged with the networking code (precisely when the thing will become threaded).

I'm almost satisfied with the performance of gui updates (thanks to mimo), so when that's done I'll subject the turnmanager to the overhaul I've been planning for a long time now. Hopefully that bug will get caught as well.

This revision was automatically updated to reflect the committed changes.