andrevmatos
commented
4 years ago The Runnable class was a temporary solution for the issue that it was (at the time) very hard to re-instantiate the whole Raiden stack. It tried to mimick a Greenlet, but greenlets can't be restarted, and need to be reinstantiated instead, so we did the Runnable thing to allow a greenlet-like interface which enabled restarting (needed for several reasons at the time).
The proper solution would be to have single-life objects, and a central (app?) factory which could [re]instantiate the whole stack from simple (e.g. serializable dict) parameters, and that would be used by cli and tests fixtures equally. The initialization and params validation was too complex at the time for this though.
The Runnable interface has start/stop methods, in theory one should be allowed to call stop and then start on the same instance, which would restart the service. This however has led to multiple, hard to debug bugs, because of improper cleanup in stop, and partial initialization in start. (E.g. Race conditions among the RestAPI and RaidenService, invalid re-initialization of the wal and WSGI server, etc.).
Additionally the Runnable interface introduce cross cutting dependencies. E.g. the stop event must be initialized as part of the start function, otherwise a stop/start cycle would not work, however, the stop_event is shared across all services, to guarantee that all services are stopped together, this means a Runnable must not clear the event, because it does not own it, meaning it is not possible to implement proper initialization in the start methods. (This led to test failures in the transport layer).