memory leak in Manager
The Manager registers a shutdown thread with the JVM, but does not unregister it after execute() is finished, which prevents the Manager from being garbage collected. run() does unregister it.
It also seems odd that the thread is registered only once (in the constructors), but never registered again after it is unregistered. The purpose of the thread is to "gracefully stop the execution of a model if the JVM is shut down (by control-C, the user logging out, etc.)", so I would expect the thread to be registered every time execution takes place.
Updated by Christopher Brooks over 6 years ago
BTW - the purpose of the shutdown hook is:
Added a shutdown hook so that model is gracefully stopped when the JVM shuts down due to control-C, the user logging out, etc. This allows 30 seconds for the shutdown, then shuts down anyway.
About leaks in Manager concerning the shutdown hook, see https://chess.eecs.berkeley.edu/ptexternal/wiki/Main/MemoryLeaks#ManagerShutdownHooks
The solution is that Manager should have a static code body and register exactly one shutdown hook.
Then Manager._registerShutdownHook should create a static list of weak references to Managers that have been created.
The one shutdown hook should check if the weak reference is valid and whether the thread is running, and if it is, call finish() and attempt a Thread.join().
I'd accept a patch that implements a fix, but I probably won't get around to fixing this myself any time soon.
Updated by Daniel Crawl over 6 years ago
Ignoring the memory leak issue, the shutdown hook does not appear to be correctly implemented since it is unregistered after the first execution (sometimes) and control-c during subsequent executions won't call it. A simple solution is to register the hook at the beginning of each model execution, and unregister when execution finishes.... this also fixes the memory leak.