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

Add signal handlers #3757

Merged
merged 12 commits into from
Jan 24, 2020
Merged

Add signal handlers #3757

merged 12 commits into from
Jan 24, 2020

Conversation

kt3k
Copy link
Member

@kt3k kt3k commented Jan 23, 2020

This PR adds signal handler APIs. Deno.signal(signo) function returns an async iterator of the given signal number. The returned object also works as a promise which is resolved when the first signal is received.

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!");

The stream can be disposed by calling .dispose() method.

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

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

The above for-await-of loop exits after 5000ms.

Note: Signal handlers doesn't block the program exiting. The following program exits immediately.

await Deno.signal(Deno.Signal.SIGTERM);

Signal handlers must be used with some other main things, such as servers, etc, and those main routines should block the program from exiting. See the discussions in #3721 #3715 for details.


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 shorthand names are inspired by methods of tokio::signal::unix::SignalKind.


Internals

This PR adds 3 ops:

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

And 1 resource:

  • SignalStreamResource

closes #2339
ref #3610 (an older version of this PR)

@kt3k kt3k force-pushed the feat/signal-handler branch from 0d04f68 to 79f5762 Compare January 23, 2020 09:59
@ry
Copy link
Member

ry commented Jan 23, 2020

@kt3k Is this working now? It looks like it.

@kt3k
Copy link
Member Author

kt3k commented Jan 23, 2020

@ry Yes, this is ready for review now. PTAL.

cc @bartlomieju @kevinkassimo

@kt3k kt3k requested review from ry and bartlomieju January 23, 2020 16:02
}

if (Deno.build.os === "win") {
testPerm({ run: true }, async function signalsNotImplemented(): Promise<
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is run permission needed here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. A mistake. Will remove.

Comment on lines 123 to 125
if (done) {
throw new DenoError(ErrorKind.NotFound, STREAM_DISPOSED_MESSAGE);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems superfluous... just return done?

Comment on lines 51 to 61
#[cfg(unix)]
#[derive(Deserialize)]
struct UnbindSignalArgs {
rid: i32,
}

#[cfg(unix)]
#[derive(Deserialize)]
struct PollSignalArgs {
rid: i32,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about single SignalArgs?

Comment on lines 167 to 170
dispose(): void {
this.disposed = true;
sendSync(dispatch.OP_UNBIND_SIGNAL, { rid: this.rid });
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe guard against additional calls to dispose()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds a good idea.

@@ -74,6 +74,9 @@ export let OP_HOSTNAME: number;
export let OP_OPEN_PLUGIN: number;
export let OP_COMPILE: number;
export let OP_TRANSPILE: number;
export let OP_BIND_SIGNAL: number;
export let OP_UNBIND_SIGNAL: number;
export let OP_POLL_SIGNAL: number;
Copy link
Member

@ry ry Jan 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better to call these

OP_SIGNAL_BIND
OP_SIGNAL_UNBIND
OP_SIGNAL_POLL

@kt3k kt3k force-pushed the feat/signal-handler branch from d52d37f to 87112aa Compare January 24, 2020 09:31
@axetroy
Copy link
Contributor

axetroy commented Jan 24, 2020

Is now able to listen to multiple signals at once?

If it can, it must be excellent

@kt3k
Copy link
Member Author

kt3k commented Jan 24, 2020

@axetroy Sorry. I haven't included that feature in this PR because this one is already a little big (to me) and I'm not so sure about the interface of that feature (Deno.signal(Signal.SIGTERM, Signal.SIGINT) ? or some other function in deno_std?). I'd like to duscuss about the interface of that feature in another issue first. (I created #3773.)

@kt3k kt3k mentioned this pull request Jan 24, 2020
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - very nice patch in the end ! Thank you for seeing this through @kt3k.

@ry ry merged commit bc89f04 into denoland:master Jan 24, 2020
@kt3k kt3k deleted the feat/signal-handler branch January 24, 2020 13:21
@ry ry mentioned this pull request Jan 24, 2020
43 tasks
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 21, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Feb 1, 2021
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
5 participants