Handle signals on separate thread

[heifner]

Currently during application startup application_base::startup, startup_thread is used to handle signals until all plugin->startup() has completed. This allows for example, ctrl-c to interrupt replay of the blocklog during chain_plugin startup.

Once startup is complete, the signal handling is moved to the main thread. This means that a ctrl-c or SIGTERM is not processed until the application_base::exec() calls io_serv.poll() on the main thread. If the main thread is busy, for example applying a large range of blocks during syncing, then shutdown has to wait until the current task completes. It requires the main thread can call poll() to process the signal to set the is_quitting atomic flag.

Instead we could continue to use a seperate thread for signal processing (except for SIGHUP which we want on the main thread since it resets all the loggers). This thread then can immediately set the is_quitting flag allowing the application to immediately exit.

[bhazzard]

This introduces another way to signal an event outside of the existing priority queue mechanism for a special case (shutting down). We may want to consider a more unified approach.

[heifner]

I missed the discussion on this, so maybe I'm not understanding the issue. What I'm suggesting we do for this issue is to have a seperate thread (just use the thread we are already using on startup) who's sole job is to call app().quit(). This approach is outside the priority queue. All extra threads are outside the existing priority queue as it is used for the main thread (and read-only threads). I don't see any reason to coordinate this with the main thread.

[heifner]

In addition, the user experience is so bad with the current implementation, I would argue this is a bug. Leap 5.0 does not require 10 minutes to shutdown when a user hits ctrl-c.

[greg7mdp]

There were multiple main discussion topics that I recall:

• whether this should be done separately or as as part of cleanup and define consistent pattern for handling nodeos plugin shutdown #842.
  I suggested that and mentioned that the fact that we do currently create the separate thread to catch signals during the plugin initialization, but then switch to catching signals in the main thread possibly had a rationale that we forgot about, warranting maybe some extra thought before reverting to use the thread throughout.
• maybe catching signals on the main thread was on purpose, because we never wanted the main thread to be stuck on a task for a long time. It made sense when calls such as get_info or get_table_rows depended on the main thread being responsive. If this is not the case anymore, maybe it is acceptable to have the main thread busy for some time and just check for quit, but not process other messages.
• however having the main thread busy for a long time and just checking for quit prevents it from processing other messages from the queue, including some which may be of a higher priority. So maybe we should have a mechanism which has the main thread checking the queue occasionally, instead of just checking for the quit boolean.

After a 1/2 hour discussion, it was decided that we needed to talk about this more with you (@heifner) present. Might very well be that the fix you suggest solves the immediate issue and should be implemented now, even if a more comprehensive solution is implemented later.

PS: Anyone, feel free to correct or add to what I recollected above.

[greg7mdp]

@heifner should we close this issue?

[heifner]

@heifner should we close this issue?

No, needed for #985