-
-
Notifications
You must be signed in to change notification settings - Fork 708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fabric server does not shutdown on stop command #3519
Comments
I cannot reproduce this shutdown issue with just Geyser-Fabric. However, this issue does occur with floodgate-fabric installed, caused most likely by bstats there. Not Geyser-Fabric though; so no idea how Geyser-fabric alone does this in your case |
In your tests, at least 1 player entered the server with minecraft bedrock (android)? |
Ah, I didn't actually test with players online, just the shutdown behavior itself. I'll see if I can reproduce it with a player join; I'll post here later |
Thanks for that info, I'm now able to replicate this: |
2.1.0-SNAPSHOT (git-master-9b3b2fb) the problem is still there |
Still there in build 238. |
Likely caused by this: GeyserMC/MCProtocolLib#699 |
jstack of Minecraft with Fabric, Geyser and Floodgate hung after a stop: |
I tried adding a shutdown of the EventLoopPool for the geyser player thread event pool. That got rid of some of the lingering threads. However, there are still some lingering threads: |
I suspect it has to do with TcpClientSocket in PacketLib creating an EventLoopGroup (static class variable) that is never shut down. See GeyserMC/PacketLib#49. |
I was looking at the wrong PacketLib implementation. I see that in the version used from MCProtocolLib there is a Shutdown hook that is supposed to close the EventLoopGroup. I will check if this is called properly. |
Shutting down the EventLoopGroup in TcpClientSession removed one of the blocking threads. |
By shutting down the EXECUTOR_SERVICE in SkinProvider as well I got a clean shutdown even after a Bedrock player had been connected. Should I file this collection of hack-ish changes as a PR? It is probably needs a lot of rework by someone that understands how the components are supposed to interact inside Geyser. |
Absolutely feel free to make a PR and we'll work from there if changes are needed. |
I created #4029 (#4029) for Geyser and GeyserMC/MCProtocolLib#744 for MCProtocolLib. I would again like to stress that these are PoC hacks to verify that it was indeed the thread pools that were keeping the server from stopping after a Bedrock player had been on the server. |
I updated the PRs above with updates as mentioned. Most important is that /geyser reload works now. |
Describe the bug
When you stop the Fabric server, the server does not shut down completely, resulting in the need to use C^ to shut down the server. This breaks automatic server restarts.
To Reproduce
Expected behaviour
The server is completely stopped and the console is closed (as it usually happens when the server is stopped)
Screenshots / Videos
No response
Server Version and Plugins
No response
Geyser Dump
https://dump.geysermc.org/P0ex2z5ML3tyXoWyJRSX5MQ2meMR8qsq
Geyser Version
2.1.0-SNAPSHOT (git-master-af5d03f)
Minecraft: Bedrock Edition Device/Version
No response
Additional Context
I downloaded the master branch and built it.
Then I added the missing mappings and languages folders to Geyser-Fabric.jar
Everything works great except /stop command (only if someone connected to the server via bedrock)
The text was updated successfully, but these errors were encountered: