Fix client hang on shutdown from non-daemon thread pools - #82
Open
deanxbox wants to merge 1 commit into
Open
Conversation
Cape repo registration and mod resource pack downloading each called Executors.newCachedThreadPool(), which spawns non-daemon threads that linger for a 60s keepalive after their task finishes. Those threads keep the JVM alive past "Stopping!", so ClientShutdownWatchdog eventually fires and reports a crash on quit. Both now use Util.nonCriticalIoPool(), the shared daemon-backed vanilla pool already used by ServerTextureDownloader. This also stops allocating a brand new pool on every call. Also drops two unused imports that were failing checkstyleMain. Fixes FrozenBlock#79
Author
|
Could we please get this merged in ? if it looks good |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #79
Problem
Quitting the game leaves the process alive until vanilla's
ClientShutdownWatchdogtimes out and dumps a crash report. The report ismisleading — there's no exception, just a
java.lang.Error: Watchdog (Client shutdown from post-main)about 15 seconds afterStopping!, with the threaddump showing nothing obviously wrong.
Cause
Two API paths call
Executors.newCachedThreadPool():CapeUtil.registerCapesFromURLModResourcePackApipack downloadingnewCachedThreadPool()usesExecutors.defaultThreadFactory(), which createsnon-daemon threads, and keeps each idle worker alive for a 60 second
keepalive after its task completes. Non-daemon threads block JVM exit, so if
you quit within that window the process just sits there until the watchdog
fires.
It also allocates an entirely new pool per call rather than reusing one.
Fix
Both now use
Util.nonCriticalIoPool()— vanilla's shared, daemon-backed pool,already used elsewhere in this repo (
ServerTextureDownloader)..forName(...)tags the worker so it stays readable in thread dumps.