Skip to content

RFC process.on("shutdown") proposal #20804

Closed

Description

@pravdomil

See this popular question on so https://stackoverflow.com/q/14031763/3748498

By now there is no clean API to do cleanup when node.js exits, in order to catch all situation you need to do following:

for (const event of ["exit", "SIGINT", "SIGUSR1", "SIGUSR2", "uncaughtException", "SIGTERM"]) {
  process.on(event, cleanUp)
}

This issue is a place for discussion how to fix it.

My proposal is process.on("shutdown", cleanUp).

Activity

  1. Fishrock123 commented on May 17, 2018

    @Fishrock123
    Contributor

    There's an npm module that does this, right? Does it need to be in core? Would it be that much more helpful?

  2. added
    feature requestIssues requesting new Node.js features.
    processIssues and PRs related to the process subsystem.
    on May 17, 2018
  3. devsnek commented on May 17, 2018

    @devsnek
    Member

    there should definitely be a One True Way to tell if node is about to exit. (as its worth noting that receiving some of those events does not guarantee that node will exit in every situation) i think we should just streamline {things in c++ that make node stop running} to always go through a call to js if possible.

  4. jasnell commented on May 17, 2018

    @jasnell
    Member

    My preference would be to make process.on('exit') always run in every case.

  5. mscdex commented on May 17, 2018

    @mscdex
    Contributor

    Changing the behavior of 'exit' could cause breakage. I would prefer a new event to avoid such issues. However there is a problem if you'd be using such a new event for "cleanup" because you wouldn't be able to do anything asynchronous, unless you additionally have a 'beforeShutdown' event or something similar.

  6. addaleax commented on May 17, 2018

    @addaleax
    Member

    There are reasons why this isn’t the default for process.on('exit') – libuv’s signal handling is asynchronous (as opposed to synchronous OS-enforced exit by default), so adding listeners for these events would avoid mean that we can’t break out of e.g. infinite loops without significantly more complex signal handling code.

  7. Horaddrim commented on May 21, 2018

    @Horaddrim

    And maybe be confusing one unique event, because if I listen to the event uncaughtException, maybe I use a different handler to a SIGTERM event. And @addaleax is extremely right to point that, maybe the assertion of this event can break some applications if you really depends on the ending of some other process.

  8. gireeshpunathil commented on Dec 1, 2018

    @gireeshpunathil
    Member

    is there an actionable on this issue? Is there any forward path justified / identified?

  9. bnoordhuis commented on Dec 6, 2018

    @bnoordhuis
    Member

    It is technically possible to make this happen but, as Anna mentions, doing it properly significantly complicates signal handling; think signal masks, watchdog threads and asynchronous termination exceptions. We'd also need to carefully audit the code base for termination exception safety.

    I'm going to say the cost/benefit ratio currently isn't worth it and close this out. If someone volunteers, we can reconsider. And who knows, maybe it'll fall out as a side effect of worker-ifying core someday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    feature requestIssues requesting new Node.js features.processIssues and PRs related to the process subsystem.wontfixIssues that will not be fixed.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions