Fix thread leak hanging shutdown on 26.2#2975
Conversation
Move Session Manager to PlatformManager and shut it down with unready event. Fix SessionManager#unload canceling timer, which prevents automatic saving of sessions after quitting a singleplayer world once.
|
I'm a bit confused by this PR, is this specifically an issue on a certain platform? I've been using Fabric with WorldEdit installed while joining a server to test the Bukkit version of WorldEdit on 26.2 and never hit this issue, and from a quick test of just launching the game in both NF & Fabric to the menu and closing it I cannot reproduce any issues. Can you please elaborate on what about 26.2 has caused this problem? |
|
Are you using the normal launcher? The This is the crash: https://pastebin.com/9kgejvG0 When using I also did some more testing and it seems the executor service doesn't seem to cause a crash, but the timer is. Either way, they should be shutdown cleanly. |
Since 26.2, the client (and possibly server?) waits for threads to exit before closing. Crashing after a short time.
This pull fixes a bug where SessionManager didn't properly shut down.
Cause:
SessionManager#unloadis never called.SessionManager#unloaddoes not shutdown the executor, causing the client to time out and crash when waiting for it to exit anyway.Fixes:
SessionManager#unloadcanceling timer and no longer auto-saving after quitting a world.SessionManager#unloadDraft reason:
I'm making this as a draft for now as I'm not entirely sure what consequences of making the session manager dependent on Ready/Unready event are.
I smoketested it on all platforms but that doesn't really say much.
Could someone with some knowledge of when SessionManager is supposed to be available chime in?
The only other thing I want some insight on was the effects of ScheduledExecutorService using an unbounded DelayedWorkQueue and the original used a bounded-queue of 5. Unless I'm missing something I don't think this will cause issues?