EngineHub / CommandHelper

Rapid scripting and command aliases for Minecraft owners
https://methodscript.com
Other
119 stars 71 forks source link

Thread leak in ReadWriteFileConnection on recompile #1389

Open PseudoKnight opened 3 months ago

PseudoKnight commented 3 months ago

Under certain conditions, ReadWriteFileConnection threads will accumulate after recompile. Affects StringSerializableDataSources (YML, JSON, INI, CSV, XML) in PersistenceNetwork. On recompile, by default all DataSources in the DataSourceFactory cache are cleared. There's a secondary cache in ThreadsafeDataSource using a WeakHashMap. If it's cleared from there by the time of next get/store_value() call, a new DataSource is created. This causes the StringSerializableDataSource to create a new ReadWriteFileConnection ConnectionMixin, which initializes a new ThreadPoolExecutor with a single core thread. This ExecutorService won't shut down until there are no core threads and all references are lost. Thus we now have two (and counting) ReadWriteFileConnection threads.

Recreation steps:

Solution depends on intended/desired behavior of these systems, but race conditions should probably be considered. Here's some ideas I explored before posting this:

This won't always affect users that use these connection types, but in the mean time users can workaround this leak by using /recompile -r to avoid reloading the Persistence Network.

PseudoKnight commented 3 months ago

Lildirt's example of the thread accumulation they were experiencing:

https://paste.thezomg.com/212182/09916317/

This behavior wasn't immediately reproduceable on a clean installation. I suspect something was triggering gc in their ms scripts, and with multiple StringSerializableDataSources, this caused these waiting threads to accumulate quickly after recompiling many times ("100+").