Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Try adding signal handler ops #3610

Closed
wants to merge 1 commit into from

Conversation

kt3k
Copy link
Member

@kt3k kt3k commented Jan 6, 2020

This PR tries to add signal handler APIs. This PR depends on tokio::signal::unix module (v0.2.6) and it supports only unix environment.

This PR adds the following JS APIs:

Streaming API

for await (const _ for Deno.signal(Deno.Signal.SIGTERM)) {
  console.log("SIGTERM received!");
}

Promise API

await Deno.signal(Deno.Signal.SIGTERM);
console.log("SIGTERM!");

and also the stream can be disposed:

const sig = Deno.signal(Deno.Signal.SIGTERM);
setTimeout(() => { sig.dispose(); }, 5000);

for await (const _ for sig) {
  console.log("SIGTERM received!");
}

The above for-await loop exits after 5000ms.

This PR also adds the following shorthand functions:

Deno.signals.alarm(); // for SIGALRM
Deno.signals.child(); // for SIGCHLD
Deno.signals.hungup(); // for SIGHUP
Deno.signals.info(); // for SIGINFO
Deno.signals.interrupt(); // for SIGINT
Deno.signals.io(); // for SIGIO
Deno.signals.pipe(); // for SIGPIPE
Deno.signals.quit(); // for SIGQUIT
Deno.signals.terminate(); // for SIGTERM
Deno.signals.userDefined1(); // for SIGUSR1
Deno.signals.userDefined2(); // for SIGURS2
Deno.signals.windowChange(); // for SIGWINCH

These short hand names are inspired by methods of tokio::signal::unix::SignalKind.


This PR adds 3 ops:

  • op_bind_signal (start watching signals)
  • op_poll_singal (poll single signal)
  • op_unbind_signal (stop watching signals)

And 1 resource:

  • SignalStreamResource

remaining tasks:

  • unit tests
  • add docs/examples
  • add all shorthand methods
  • update lib.deno_runtime.d.ts
  • fix panic at sig.dispose()
  • for-await loop doesn't end after sig.dispose() -> now it works.
  • modify handling of pending_ops

First I started with the API design loosely following the suggestion by @sholladay at the comment ( #2339 (comment) ), and then I changed it a little inspired by tokio's naming of SignalKind shorthands https://tokio-rs.github.io/tokio/doc/src/tokio/signal/unix.rs.html#82


closes #2339

@kt3k kt3k force-pushed the feature/signal-handler branch 18 times, most recently from 6039c1c to 515afda Compare January 10, 2020 23:49
@axetroy
Copy link
Contributor

axetroy commented Jan 11, 2020

Should we support listening for multiple signals?

I think this is still useful, ref Golang

quit := make(chan os.Signal)
signal.Notify(quit, os.Kill, syscall.SIGHUP, syscall.SIGINT, syscall.SIGTERM, syscall.SIGQUIT)

go func() {
	<-quit
        // do staff
	os.Exit(1)
}()

defer signal.Stop(quit)
for await (const signal for Deno.signal(Deno.Signal.SIGTERM, Deno.Signal.SIGINT, Deno.Signal.SIGHUP)) {
  console.log(`receive ${signal}`);
}

@kt3k
Copy link
Member Author

kt3k commented Jan 11, 2020

@axetroy I prefer to keep Deno.signal to handle only one signal because the underlying tokio API only supports one kind of signal for each call. I think we can provide that kind of API in std.

@kt3k
Copy link
Member Author

kt3k commented Jan 11, 2020

I'm a little stuck at the task modify handling of pending_ops (We need to ignore pending poll_signal async ops when checking the poll result of pending_ops, which is what Kevin did in #2735 with AsyncOptional ops). I think that task requires a lot of code change. Maybe it's better to try that task alone first (That can be done independently of this PR). Except that point, I think this PR is almost finished.

@kt3k kt3k mentioned this pull request Jan 19, 2020
@kt3k kt3k closed this Jan 21, 2020
@kt3k kt3k deleted the feature/signal-handler branch January 21, 2020 17:16
@kt3k kt3k mentioned this pull request Jan 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signal Handlers
2 participants