Fix players timing out after a command executes for a while

[neeleshpoli]

Description

Whenever the fill command is run with a large selection, the client times out. This is because the packet processing is halted because it is processing the command. The best way to solve this would be to process the command in a new task so that packet processing in not halted for commands that take longer to run. This should stop the client from timing out when they run a command that has longer run time and if they do time out while a command is running for whatever reason, the client will properly be removed from the server.

Testing

Forcibly cause a timeout (by an extremely large selection with the fill command), and make sure that the player can rejoin the server.

Please follow our Coding Guidelines

[kralverde]

Canceling tasks is done in the remove_player method. I do not like this fix and if this works it is evidence of a deadlock occurring in the tokio runtime when a timeout happens. look into that code instead

[neeleshpoli]

Canceling tasks is done in the remove_player method. I do not like this fix and if this works it is evidence of a deadlock occurring in the tokio runtime when a timeout happens. look into that code instead

Yeah you're right, its more of a band aid fix. I'll see if I can find the deadlock 👍

[neeleshpoli]

Ok after some debugging, there is no deadlock, it's just hanging at the process packets stage for some reason 🤷‍♂️

[neeleshpoli]

After some more debugging, I found the actual issue. So whenever the fill command is ran with a large selection, the client times out. This is because the packet processing is halted because it is processing the command. The best way to solve this would be to process the command in a new task so that packet processing in not halted for commands that take longer to run. This should stop the client from timing out when they run a command that has longer runtime and if they do time out while a command is running for whatever reason, the client will properly be removed from the server.

[kralverde]

If it's cpu intensive, use a rayon task (but use an async waiting method if you need to wait on it) otherwise use a tokio task

[neeleshpoli]

If it's cpu intensive, use a rayon task (but use an async waiting method if you need to wait on it) otherwise use a tokio task

Whenever a command is ran, a new tokio task will be spawned now since they are all async anyway. If it needs to, the command itself would have to spawn a rayon task.

[Snowiiii]

What about Commands executed in Console or RCON ?

[neeleshpoli]

What about Commands executed in Console or RCON ?

Oh yeah, I didn't think about that. I can fix those too.

[Snowiiii]

I'm not sure, but can't/should we just spawn a new tokio thread in the dispatcher::handle_command function ?

[neeleshpoli]

I'm not sure, but can't/should we just spawn a new tokio thread in the dispatcher::handle_command function ?

Yes we could, but however there are many lifetime issues to resolve before that's possible. Thats why I spawned a task around the handle_command function instead.

[neeleshpoli]

Seems like something was pushed to master which also broke CI. I force pushed my branch back.

[Snowiiii]

I mean for now its fine. Thank you @neeleshpoli 👍