Add Process.on_terminate

[stakach]

This fixes some inconsistences between the platforms

• previously the unix implementation was not ignoring the interrupt whilst the windows version is.

Also improves usefulness

• handles ctrl-c, terminal closed and user session ending interrupts

Replaces #13654 as I screwed up my rebase

Resolves #13983

[stakach]

@HertzDevil @straight-shoota what do you think?

[m-o-e]

This impl suffers from the same problem that is described for Signal#trap here: #13803

[HertzDevil]

This PR is independent from #13803, although the relative positions between the previous handler parameter and the reason parameter could introduce a breaking change if not done carefully.

[stakach]

Any thoughts on this one? Would be nice to simplify this situation:

  {% if flag?(:win32) %}
    Process.on_interrupt { shutdown_gracefully }
  {% else %}
    # Detect ctr-c
    Signal::INT.trap { shutdown_gracefully }

    # Docker containers use the term signal
    Signal::TERM.trap { shutdown_gracefully }
  {% end %}

[straight-shoota]

It would be preferrable to discuss the problem in an independent issue and then think about potential solutions. Being presented with an implemented solution without a detailled and open analysis of the issue it tries to solve may hinder the problem solving process. It's great that you already did the impementation work. And making concrete suggestions is certainly helpful. But a pull request with a concrete implementation isn't a great place to discuss solution strategies openly.
For this reason we ask to open an issue first before opening a pull request.
This topic might seem trivial, but I don't think it is. At least not the part of integrating it into the existing API (and pending parallel API changes like #13803).

The previous comment shows a great example of why this is useful. I've created issue #13983 so we can talk independent of this particular implementation. Until that's come to a conclusion, this PR should be a draft.

[straight-shoota]

thought: IMO it would not necessary to deprecate this method. It's just the backend of Process.on_interrupt and should never be called directly. It doesn't hurt to have the deprecation, though.

[stakach]

yeah I put it there so it would be simpler to find and remove at a later date. Let me know if you want me to remove

[straight-shoota]

No, I think it's fine. Thanks!

[stakach]

@HertzDevil what do you think?

[stakach]

@HertzDevil any further changes you can think of?

[crysbot]

This pull request has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/upcoming-1-12-0-release/6714/1