Fabric with worldedit hangs when stopping during shutdown

[petersv5]

WorldEdit Version

7.3.0-beta-03

Platform Version

Fabric Loader 0.15.3

Confirmations

• I am using the most recent Minecraft release.
• I am using a version of WorldEdit compatible with my Minecraft version.
• I am using the latest or recommended version of my platform software.
• I am NOT using a hybrid server, e.g. a server that combines Bukkit and Forge. Examples include Arclight, Mohist, and Cardboard.
• I am NOT using a fork of WorldEdit, such as FastAsyncWorldEdit (FAWE) or AsyncWorldEdit (AWE)

Bug Description

The server hangs during shutdown in some cases.
The was tracked down to the thread "WorldEdit Task Executor - 0" left running, whichin turn is due to the WorldEdit.executorService not being stopped. This particular task executor service is not run using daemon threads and thus require an explicit shutdown to terminate. If not stopped it will prevent the jvm from initiating the shutdown.

Expected Behavior

Sending /stop to the server should terminate the process in a reasonable time.

Reproduction Steps

1. Make a selection
2. //copy
3. //schem save somefilename
4. /stop

Observe that the fabric server does not termiante.
If checking the remaining server status with jstack the thread "WorldEdit Task Executor - 0" is still running and not a daemon thread.

Anything Else?

I am preparing a PR for this, hopefully later today.

[petersv5]

It seems that in addition a timer is created during shematic saves that is not cancelled. This leads to a thread "Timer-1" from java.util.TimerThread that also remains live through the shutdown. I'm still tracking this down before filing the PR.

[octylFractal]

Realistically for safety I believe that we should be properly shutting down the executor service on mod/plugin unload, not just making these daemon threads.

The Timer should automatically be GC'd after the schematic save is complete. If a reference remains, that is what should be fixed. I would be interested in a heap dump to see where this is going wrong.

[petersv5]

The executor service should be properly shut down by the PR, that was not made a daemon thread.

The Timer is a bit funny. The actual timer that resolved the hang when it was made a daemon thread is only ever used by the FutureProgressListener constructor. I see an equal number of construltor calls and calls to the run() method which in turn cancels the timer. So in theory there should be no timer reverences anywhere. At the point of the hang all non-daemon threads are already dead except "Timer-1". Either one of the daemon threads hold the reference to the FutureProgressListner Timer or it holds it itself.

[petersv5]

The Timer timer in FutureProgressListener is a static field. It is not going to go away, I guess. Is it actually correct to use a single Timer shared by all instances of FutureProgressListener? That should make them interfere with each other, I think.

There is another Timer in worldedit SessionManager that also keeps the TimerThread alive.

For the Timers it may actually be a better idea to make the threads a daemon. It is unlikely that any timer callbacks can do much good once the server has shut down which it will have done by the time the daemon-ness of the timer (or not) matters.

I am trying to make the timers go away by making some changes:

• the FutureProgressListener timer was made non-static field.
• the SessionManager timer was explicitly nulled on unload.

I still sometimes see the TimerThread being kept alive by Timer@ThreadReaper for a long while, but it eventually goes away. Still, this delay is a can be a problem for servers.

I will look a bit more at this tomorrow.

[stale]

This issue has been automatically marked as stale because it has not been fully confirmed. It will be closed if no further activity occurs. Thank you for your contributions.

[petersv5]

Will test latest and get back if it is still an issue.