Investigate race condition around state changes

[Eluinhost]

Connecting to a created server with a regular minecraft (1.7.9) throws an error about the UUID being 0. I think it has to do with this note:

'As of 1.7.6 all UUIDs used in the protocol now contain '-'. The session server still returns them without'

I did the following change to fix for my particular use case but being new to nodejs and this project too I have no idea of the 'proper' fix for this

Eluinhost@16681cf

On the same kind of note, when I go to connect a lot of the time I get a ClassCastException 'bll cannot be cast to fv'. I don't know how to fix this one but there is a mojang ticket here: https://bugs.mojang.com/browse/MC-42286 that says the Keep Alive packet may be being sent too early causing Minecraft to choke

[roblabla]

Nice catch. I just looked at my UUID code, and yeah, the problem is indeed with the stupid '-'s.

The KeepAlive problem, though... I'm not too sure. Looking at the code, we're sending keepalive packets right after the login procedure is finished, AKA when the player entered the "Play" phase, so I don't see this being an issue. You could try de-activating the keepalive entirely, see if you can login at all.

I'll investigate some more.

[Eluinhost]

I managed to get the debug on to see all the packets. I'm kicking players when they login successfully with a message, the times the player gets the ClassCastException and the times they don't have 0 difference in packets sent. Adding a 3 second delay before actually kicking them seems to make it work 100% of attempts. Here is the file (at the current commit) that I am using in case I'm making some kind of newbie JS error - https://github.com/Eluinhost/TeamspeakAuth/blob/d211b9453b715cf43a63bcd7815f5876be4c789b/authserver/AuthServer.js

[roblabla]

It looks fine. I'm assuming it crashes if https://github.com/Eluinhost/TeamspeakAuth/blob/d211b9453b715cf43a63bcd7815f5876be4c789b/authserver/AuthServer.js#L121 is removed ?
I'll have more time to look at this over the week-end.

[Eluinhost]

Nothing seems to happen strange server side, just a lot of the time (but not always) the client gets the cast exception message instead of the kick message. I'm assuming its related to the keep alive error the link was talking about where the client is casting the Login to a Play. I think the client is casting the login to a play when it receives the kick packet because it expects it when it is in Play stage (which the server is in, but sometimes the client is still in Login and probably made easier to break by it all being on localhost). Delaying with the sleep allows the client some time to switch to Play before getting the packet and it works fine

[rom1504]

If this is still relevant I guess it should be in a PR, @Eluinhost ?

[roblabla]

What we seriously need to do is process.nextTick() the event emitted when changing state. This probably would avoid a lot of race conditions when doing code like

client.on('state', function(newstate) {
  if (newstate === states.PLAY)
    client.write(/*stuff*/);
});

The above crashes the server due to the server not being in PLAY state yet. Using process.nextTick seems to fix the issue.

[rom1504]

hmm would this happened to be solved now that #157 is solved ?

[roblabla]

Don't think so, this is a race condition from the notchian stuff that we have to manually work around ^^

[rom1504]

The title of that issue should be changed. I'm not sure exactly what the new title would be ("Fix race condition when changing state" maybe ?), but the current one doesn't describe the problem we're trying to solve here, and it's confusing.

[rom1504]

That kind of thing should not happen unless we have an async read/write/sizeOf (see ProtoDef-io/node-protodef#49 and #309 (comment) )

I believe it doesn't happen in current nmp, so closing this. We can always reopen if there are new problems related to this.