Repository navigation
✨ feat(server): implement graceful exit handling with log flushing - #449
Conversation
twisti-dev
commented
Oct 17, 2026
twisti-dev
commented
- add exitFlushingLogs function to stop server and flush logs before JVM termination
- update SurfMinestomServer to use exitFlushingLogs for graceful shutdown
- introduce MiniPlaceholdersCommandMixin for compatibility with MiniPlaceholders 3.2.0
- update gradle.properties to version 3.50.0 and add new dependencies
- add exitFlushingLogs function to stop server and flush logs before JVM termination - update SurfMinestomServer to use exitFlushingLogs for graceful shutdown - introduce MiniPlaceholdersCommandMixin for compatibility with MiniPlaceholders 3.2.0 - update gradle.properties to version 3.50.0 and add new dependencies
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 523ab970e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| private fun terminate(): Nothing { | ||
| // Waits for a start in progress, so that the server it starts is stopped as well | ||
| current.get()?.lifecycleLock?.withLock {} |
There was a problem hiding this comment.
When exitFlushingLogs is requested from another thread while start() is in progress, this empty lock acquisition can finish immediately after the startup thread releases lifecycleLock but before it installs the shutdown hook and assigns lastStarted. The exit thread then observes no runtime to stop and calls System.exit, so feature/plugin teardown and registered shutdown actions are skipped, potentially losing data they were expected to flush. Retain and stop the current runtime after waiting, or publish the runtime and hook before releasing the lifecycle lock.
Useful? React with 👍 / 👎.
| override fun exit(status: Int) { | ||
| Thread({ | ||
| try { | ||
| exitFlushingLogs(status) |
There was a problem hiding this comment.
When two callers request different exit statuses close together, invocation order is not preserved because this method only starts a thread and exitRequest.compareAndSet runs later inside that thread. A later direct exitFlushingLogs(0) or a more promptly scheduled exit(0) can therefore win over an earlier exit(1), causing a failed server to report a successful process status despite the API promise that the first request wins. Record the request synchronously before starting the asynchronous termination work.
Useful? React with 👍 / 👎.
| Runtime.getRuntime().addShutdownHook(Thread({ | ||
| try { | ||
| stop() | ||
| } finally { | ||
| LogFlush.flush() | ||
| } |
There was a problem hiding this comment.
After shutdown() followed by another supported surfMinestomServer() start, both runtimes retain shutdown hooks. On a signal those hooks run concurrently; if the active hook wins Minestom's one-shot stop, the older hook can return from stopCleanly() immediately and reach this flush while the active runtime is still disabling features or running shutdown actions. Shutting Log4j down at that point drops the remaining shutdown logs, so use a single global hook targeting the latest runtime or otherwise coordinate one final flush after all runtime hooks finish.
Useful? React with 👍 / 👎.
