Skip to content
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

fix: make sure Minestom services stop like Bukkit services #1338

Conversation

GiantTreeLP
Copy link
Member

Motivation

Minestom is a very lightweight framework for implementing specialized Minecraft servers.
Especially when the full implementation or behavior of vanilla Minecraft is not needed, Minestom offers a way to run services with very little resources.

One issue that arised in conjunction with CloudNet is that services implemented using Minestom would not properly stop. Minestom would shut down and the service would get unregistered from CloudNet but the processes would be kept running.

Modification

This pull request mimics the behavior of vanilla implementations of the Minecraft Server which all call System.exit(0); when their process is done. This change makes sure that the JVM shuts down properly, even if other threads might block an ordinary shutdown from happening (ie. Netty and other networking related tasks).

Result

Services using the Minestom framework now properly shut down and allow CloudNet to advance to the next lifecycle steps.

Other context

Fixes #1304

This change implements a class transformer for net.minestom.server.MinecraftServer to call System.exit when MinecraftServer.stopCleanly is called.
@GiantTreeLP GiantTreeLP changed the title feat: Make sure Minestom services stop like Bukkit services Fix: Make sure Minestom services stop like Bukkit services Dec 6, 2023
@GiantTreeLP
Copy link
Member Author

GiantTreeLP commented Dec 8, 2023

As of now, this solution also works on server implementations build on hollow-cube/minestom-ce.

@0utplay
Copy link
Member

0utplay commented May 16, 2024

Can we maybe implement this as discussed here (https://discord.com/channels/713041279277203457/777861579659411466/1182967379488161872)

@GiantTreeLP
Copy link
Member Author

Can we maybe implement this as discussed here (https://discord.com/channels/713041279277203457/777861579659411466/1182967379488161872)

I'm going to update my implementation to match what we discussed back then.

This change implements a class transformer for net.minestom.server.ServerProcessImpl to call System.exit when ServerProcessImpl.stop is called.
The changes also take care of correctly patching the opcode, if label nodes follow the last return statement.
@GiantTreeLP
Copy link
Member Author

GiantTreeLP commented May 16, 2024

@0utplay I have updated the PR but can't initiate the build required.

Feel free to also add a review to the code, the changes are pretty minimal.

@0utplay
Copy link
Member

0utplay commented May 16, 2024

Thats because there are conflicts in the Wrapper class. I can resolve them later.

…ut-down-transformer

# Conflicts:
#	wrapper-jvm/src/main/java/eu/cloudnetservice/wrapper/Wrapper.java
@0utplay 0utplay changed the title Fix: Make sure Minestom services stop like Bukkit services fix: Make sure Minestom services stop like Bukkit services May 16, 2024
@0utplay 0utplay changed the title fix: Make sure Minestom services stop like Bukkit services fix: make sure Minestom services stop like Bukkit services May 16, 2024
GiantTreeLP and others added 2 commits May 17, 2024 17:24
This change improves the quality of the MinestomStopCleanlyTransformer by making it final and handling the unlikely case, that there is no return statement in the method to be patched.
@0utplay 0utplay merged commit d3650aa into CloudNetService:nightly May 17, 2024
3 checks passed
@GiantTreeLP GiantTreeLP deleted the 1304-minestom-does-not-shut-down-transformer branch May 17, 2024 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minestom server doesn't shut down correctly
2 participants