Close threads at server shutdown

[petersv5]

To exit normally all non-daemon threads of the java process are supposed to have stopped. There are several cases of EventLoopGroup-created thread pools that are not shut down as a part of the server shutdown.

These threads are spawned on the first Bedrock player login. The identified cases are:

• TcpClientSocket created by GeyserSession
• The listener threads in GeyserServerInitializer
• The player threads in GeyserServerInitializer
• The EXECUTOR_SERVICE in the SkinProvider

This patch depends on the PR
GeyserMC/MCProtocolLib#744 submitted to MCProtocolLib.

This is a very quickly thrown together proof of concept to verify that the execution thread pools left running were what blocked a clean exit with Geyser active.

[Camotoy]

Can we make sure this works with /geyser reload?

[petersv5]

Can we make sure this works with /geyser reload?

Will that call GeyserServer.shutdown()? If so things will break for the cases where the EventLoopGroup is held in a static class variable and some other approach will be needed.

[Konicai]

Spigot's reload should also be tested (despite it being terrible) to make sure things don't get any worse than they already are

[petersv5]

Like I wrote in the MCProtocolLib PR, it is probably best for someone that knows Geyser to reimplement these changes properly. This patch was literally created with grep, jstack and jdb.

[petersv5]

I can confirm /geyser reload is completely broken, as expected.
I'll make a version that actually unsets the variables tracking the event loop groups which should allow them to be recreated.

However, I wonder if the better solution would be to make the event loop groups instance variables instead of class members. I think we only create one instance of any of the classes holding event loop groups so there is no real point in making them static class variables. We never reuse them anyway. I need to go over all of them, but I think we never create many of them. At least not on a regular server. Are there cases with multiple instances in one process and where it is desireable to share worker pools?

[petersv5]

I pushed an update that allows /geyser reload to work.
The patch now nullifies EventLoopGroup holders when shut down.
EventLoopGroup users that are class variables can automatically restart the EventLoopGroup on first use. This affected TcpClientSession (see the updated patch to MCProtocolLib) and SkinProvider. The remaining EventLoopGroups were instance variables of objects that were recreated after a /geyser reload.
I also changed the name of player_group to playerGroup as requested.

I am not able to test spigot reload as I do not have such a system set up anymore.

[petersv5]

Rebased to latest master.

[petersv5]

Rebased to latest master

[petersv5]

Switched to using the alterantive PR for ProtocolLib (GeyserMC/MCProtocolLib#748) after comments from Camotoy. This one no longer has an explicit shutdown of the TcpClientSession EventLoopGroup. That is now reaped on process exit.

The remaining EventLoopGroup threads are still shutdown on Geyser shutdown.

I have tested /geyser reload and that seems to work.

[petersv5]

Rebased to latest master.

[petersv5]

Rebased

[petersv5]

Rebased and tested

[Konicai]

thank you

[onebeastchris]

Tested on all platforms (compiled locally with the mcpl pr), works perfectly! Thanks for this fix & for keeping the PR updated :)

[Konicai]

Until 1.20.2 when we can target a newer MCPL

[Konicai]

thanks again!