I have finished typing! Reopening!

It's worth noting that SIGINT doesn't necessarily mean the program will exit. It means the system has requested that the program interrupt whatever work it's actively doing. For example in the REPL, sending SIGINT stops whatever is currently evaluating, it doesn't close the REPL. This is also why SIGINT doesn't guarantee the exit event.

I think a key point here is that SIGINT is top-level, big public behavior, so individual functions shouldn't have a concept of SIGINT for cleanup, since other things could also cause cleanup. You'd want to bundle that all into some sort of "cleanup" interface and then flow your various conditions for cleanup into that.

Also, there is also some work going into defining better ways of cleaning up resources: https://github.com/tc39/proposal-explicit-resource-management

The specific example of SIGINT not causing is this:

console.log(`PID ${process.pid}`);

process.on("exit", (code, signal) => {
  console.log("exit called!", { code, signal });
});

setInterval(() => {
  console.log('Still alive!')
}, 1000);

setTimeout(() => {
  console.log("Goodbye!");
  process.exit(0);
}, 10000);

process.stdin.resume();

If you run this and then send Ctrl-C (or kill -2), it does not fire the exit event. Same for SIGTERM.

Also, there is also some work going into defining better ways of cleaning up resources: https://github.com/tc39/proposal-explicit-resource-management

I was there for the presentation, but I don't think it helps. It's not that try/finally in JavaScript does not work in correctly in general, it's that signals and exits don't cause an exception (or return, or any other kind of completion event) from the perspective of the async function, so the finally block is not called. I don't think the proposal changes anything in this regard.

your example there is interesting. in general we can't fire an event for sigint exits if js is still running, the idea is to interrupt it after all, and js doesn't really work with reentrancy. I would, however, expect the event to be fired it if happens while waiting on the event loop. Perhaps it doesn't for consistency reasons.

I would, however, expect the event to be fired it if happens while waiting on the event loop.

Right, that's why I added the setInterval to illustrate that it is not stuck in some blocking sync code.

I think a key point here is that SIGINT is top-level, big public behavior, so individual functions shouldn't have a concept of SIGINT for cleanup, since other things could also cause cleanup. You'd want to bundle that all into some sort of "cleanup" interface and then flow your various conditions for cleanup into that.

What should libraries do?

Let's say, you want to provide a withTmpDir that creates a temp directory, pass it to the user's callback, and the remove the temp directory afterwards. Something like this:

async withTmpDir(callback) {
  let dir = createTmpDir();
  try {
    return await callback(dir);
  } finally {
    removeTmpDir(dir);
  }
}

We already know that this doesn't cleanup the temp directory if you exit/SIGINT/SIGTERM before callback resolves/rejects, the question is, if it's really import for this function to remove the temp directory "whenever possible" – namely handling "graceful" exit/SIGINT/SIGTERM, is there a way to do it correctly, and so far my conclusion is there isn't.

The nature of the cleanup work here isn't really that important, you can substitute "removing a temp directory" with a different task, like shutting down detached child processes, closing database connections, etc.

It would be great if Node can provide the coordination needed to "bundle that all into some sort of "cleanup" interface. If the exit event does fire on SIGINT and SIGTERM, that is perhaps sufficient for this particular case.

it's that signals and exits don't cause an exception (or return, or any other kind of completion event) from the perspective of the async function

ah i see the pattern you're going for now. yes, we wouldn't deliver such occurrences as exceptions because of a sort of "cardinal rule" which the ecosystem generally follows which is that the world isn't supposed to mutate while js is running, you'd have to wait until after the js finishes running to know if the outside world requested that the js stop running.

What should libraries do?

It's a good question. I've had the same issue working on libraries that mount virtual filesystems, and will easily crash the OS if not unmounted correctly. Lots of things actually have this issue, for example I have vim set up to run eslint automatically, and every once in a while eslint leaves some orphaned processes behind because i killed vim. Here is prior art on trying to make this less messy that I searched up, hopefully it can provide additional information: #20804 #2853 #9050

There's no real solution here. Either you accept that:

1. the process terminates immediately, or that

2. termination can be postponed indefinitely because cleanup code can spawn new tasks, which spawns new tasks, which spawns new tasks, ad infinitum.

If (2) is an acceptable middle ground for you, then you can already accomplish that through the process.on('beforeExit') event.

The one thing Node.js can (perhaps/arguably) do better is wait for promises to resolve on normal exit, when there is no more work to do.

That's being discussed in #29355 and doesn't need changes to Node.js core, strictly speaking.

Even if you accept (2), or a softer variant of (2) (up to some time limit, etc), I'm don't think it can really be done today.

For one thing, signals and explicit exit does not emit 'beforeExit'. It's still the case that I believe there is no way to "do some cleanup work if the SIGINT is going to cause termination, but do nothing otherwise".

signals and explicit exit does not emit 'beforeExit'.

For the record: that's by design. process.exit() means "stop right now."

SIGINT works the same way (unless you handle it) because that's the traditional/expected behavior for a UNIX application.

signals and explicit exit does not emit 'beforeExit'.

Correct, 'beforeExit' is not named well. It should be something like (on) 'eventLoopEmpty'.

There's no real solution here.

Well, the "real solution" here is promise cancellation, which would have solved this - unfortuunately that was rejected by tc39 for a more explcit proposal that does no let us do this.

Other languages do "creative" things like running finally blocks on generator GC (ahem python).

I would also like to challenge the assumption we can't fix this - we can (as the platform and from the V8 side) throw an AsyncFunctionInterruptedError error into the current async function which would cause the finally to run. Unfortunately without third-state semantics that would also cause catch blocks to run. I don't think we can do this without breaking JavaScript which none of us want to do. I think we need to take this use case up to TC39.

On a side note good issue and reproducing example @chancancode thanks 🙏

The more I think about this the more I become convinced this is a footgun in how we deal with async functiions. If in this case:

async function someTask() {
  try {
    await someOtherTask();
  } finally {
    // MUST PERFORM VERY IMPORTANT CLEAN UP!
  }
}

Sending a SIGINT (when no SIGINT signal handler is set up) could reject the promise returned from someOtherTask() which would cause the cleanup to run.

I agree that we should continue looking for creative ways to solve this. Generally though, we should avoid growing the event loop (in fact we ignore new tasks during a normal exit), so i don't think we can rely on the promises themselves in the exit flow.

@devsnek I am really not sure what we can or should do but I am really not a fan of the user stories in our current behavior and I would love to see how this can be fixed from the language side and not from the platform side because this is essentially the same issues web-pages have if the user navigates from them during an operation or when a browser stops running JavaScript on a page to save power.

Perhaps one route could be registering https://github.com/tc39/proposal-explicit-resource-management scopes in a big list and calling them. We still end up with the problem that we can't dispatch this in a sync context due to design and implementation choices, and we can't dispatch this in an async context because we can't put tasks on the queue...

we still end up with the problem that we can't dispatch this in a sync context due to design and implementation choices, and we can't dispatch this in an async context because we can't put tasks on the queue...

I mean, I get why we might not want to do either and I am not even sure we should propose either (I opened an issue in that repo) but I am not sure why we can't do either?

the decisions made in the past are: we don't want to do it synchronously because the state of the world can't change while js is running. we don't want to do it asynchronously because queuing more tasks means the process lives longer and we're trying to end the process.

Obviously we can change our minds about these decisions, but they've been in place for a while.

Ok, so I think I understand, your point is that cleanup for stuff like this is typically asynchronous in nature anyway and we cannot (safely) delay the process exiting by waiting for asynchronous actions to happen. That makes sense and I agree.

We theoretically could also have different behavior for a "clean" exit (like a SIGINT) and a "dirty" exit (like a process.exit(1)) but given that cleanup is "best effort" anyway I can totally also see not wanting to do anything here anyway.

It appears that Python does the same thing we do in async functions. (You would do loop.add_signal_handler() on your event loop and handle it there but it doesn't get stuff thrown onto the async function). In C# you would be expected to pass a CancelToken throughout your app in a patten that looks like this:

const token = { cancel() { throw new CancelError('cancelled')} };

async function someTask(token) {
  try {
    await someOtherTask(token);
  } finally {
    // MUST PERFORM VERY IMPORTANT CLEAN UP!
  }
}

And your interrupt handler would be process.on('SIGINT', () => token.cancel()).

And users would be expected to know to filter the CancelError (well CancelException since this example is in C#) in their instrumentation and logs.

I think at its heart this issue is about the guarantees provided by the finally clause, and therefore about its suitability for important resource cleanup tasks, especially after unexpected failures.

In Python, we love using the finally clause precisely because of its guarantees. The code you put in there is run, and it can be an incredibly powerful and easy-to-reason-about technique.

I am still new to this ecosystem, but the question whether I should do the same or not in NodeJS/JavaScript was one of the first ones I researched more deeply. I found the following discussion to be incredibly insightful: https://esdiscuss.org/topic/garbage-collection-in-generators

Andreas Rosberg said about

function getResults*() {
     try {
         var resource = acquire();
         for(const item of resource) yield process(item);
     } finally {
         release(resource);
     }

that it is a

"bogus forms of resource management". This is an anti-pattern in ES6. It won't work correctly. We should never have given the illusion that it does.

and Benjamin Gruenbaum (@benjamingr) was kind of representing my perspective in this discussion, saying that

try/finally is commonly used for resource management and garbage collection is a form of automatic resource management. There is also the assumption that finally is always run. Two other languages have opted into the "run finally" behavior so I wouldn't call it crazy

and that

I'm very curious why languages like Python have chosen to call finally blocks in this case - this was not a hindsight and according to the PEP. They debated it and explicitly decided to call release.

The fundamental difference between the approaches might be as of garbage collection, seemingly, as Mark S miller says:

JavaScript GC is not specified to be precise, and so should be assumed conservative. [...] C++ RAII and Python refcounting are completely different: they are precise, prompt, predictable, and deterministic.

but on the other hand Andreas Rossberg said

Try-finally has nothing to do with GC, it's just control flow

I don't think the discussion was concluded in a really satisfying way, but my big takeaway was one should not rely on the finally clause in generators.