Initial implementation of general user land signal handling (unix only)

[kevinkassimo]

Implement general signal handling sigaction(signo, handler).

Try it out by pressing Ctrl-C when running this script (wait for TS compiler to finish compile first, since it is not immune to SIGINT):

Deno.sigaction(Deno.Signal.SIGINT, () => {
  console.log(">> SIGINT caught (1)");
});

Deno.sigaction(Deno.Signal.SIGINT, () => {
  console.log(">> SIGINT caught (2)");
});

console.log("sigaction() DEMO starts");

setTimeout(() => {
  console.log("DONE");
}, 5000)

Notice that signal handler would not block script from exiting after timeout, and multiple handlers could be registered.

Achieved by implementing optional Op type which does not prevent process exit.

Code is hacked up so needs much polishing after confirming the approach. API design feedback also needed.

Tests needed. Added basic unit test

[ry]

Hmm - what is the purpose of this? I'm not open to adding to this enum...

[kevinkassimo]

It is a marker such that when later registering the op we can tell if the op is optional and should not block event loop exit (by later incrementing optional_ops_count on Isolate). It is used since I don't think there is some easier way to pass this bit of information down other than with a new enum (otherwise we need to change the signature of Async)

[kevinkassimo]

The design of signal is by first calling SignalListen to add a signal stream to resources. Then signal is actually handled by repeatedly calling SignalPoll to wait for next poll of signal -- somewhat similar to what we do with TcpStream read. But unlike reading from TcpStream, the task registered by SignalPoll is optional, such that if all other required tasks has finished (pending_ops only contains optional ops), program would exit right away.

[ry]

Can you explain? I don't get it.

[kevinkassimo]

It actually act almost the same as Op::Async for this part, only that the result wrapping enum is different. I'll see a way if I could avoid duplicate code here

[piscisaureus]

What's the purpose of this extra variant?
To make sure the process will exit even when the future is not ready?
I'd suggest to use just Op::Async() and have a separate pending-op count rather than using pending_ops.len().

[kevinkassimo]

Maybe I'm misunderstanding, but if both are marked as Op::Async I don't think Isolate will be able to tell if a future is optional when polling?

I kind of want to avoid manipulating Isolate directly from cli (and I'm not quite sure if changing return types of op selectors is a good idea). Also not sure about the counter part since no differentiation between things like enum types mean there is no way I could just decrement for required futures, unless we want to chain another step after the user provided future -- and thus also need to make the counter atomic

[ry]

We need something like uv_unref ... I’m going to punt on this problem until next week - not sure what the right way to handle this is.

[kitsonk]

My biggest feedback is I am personally not convinced by sigaction. Why wouldn't Deno.signal do? To me the intent would be clear. If not, than something like onSignal.

[kevinkassimo]

@kitsonk Deno.Signal is currently the enum of the signal numbers (e.g. Deno.Signal.SIGSEGV). I am worried that introducing another Deno.signal of lowercase would be confusing.

Also Deno.signal sounds more like op to send a signal ("signal a process to handle something") instead of receiving (which we currently have as Deno.kill. I know Python uses the name signal.signal but I still found it quite unintuitive). Both kill and sigaction might be confusing to people with pure frontend background, but they do comply with the Unix traditions...

That being said, except for that I don't think Deno.signal would be good name for registering signal handlers, I am okay with most other names or other API design choices. The interface I showed there is just an POC and for feedback purposes.

[ry]

I think we should just stick with the POSIX names to avoid confusion. Most people should not need to ever look at signal stuff.

That said, the issue with this patch is rather the lack of uv_unref() in tokio...

[ry]

Closing because this is old. We still don't have a solution to uv_unref...