CommandHelper

CommandHelper

46.5k Downloads

Thread leak in ReadWriteFileConnection on recompile

PseudoKnight opened this issue ยท 1 comments

commented

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:

  • Setup JSON (or other StringSerializableDataSource) connection in persistence.ini
  • get|store a value in this DataSource to create core thread in ReadWriteFileConnection
  • Recompile to clear data sources from DataSourceFactory static map
  • Force a gc to clear data sources from ThreadsafeDataSource static map
  • get|store same StringSerializableDataSource again

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:

  • Make ThreadPoolExecutor static in ReadWriteFileConnection (only ever one thread for these connection types)
  • Shutdown ThreadPoolExecutor in a new method that's called in StringSerializableDataSource.disconnect() and then await termination to ensure all tasks are complete
  • Set core threads to zero (simple, but doesn't help with race conditions)
  • Do not clear DataSources from DataSourceFactory (would keep around DataSources for removed connections, but this is rare; would need to make sure disconnect() doesn't break any DataSources)

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.

commented

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+").