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 Deno.kill(pid, signo) and process.kill(signo) (Unix only) #2177

Merged
merged 4 commits into from
Apr 22, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions cli/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ main_extern = [
if (is_win) {
main_extern += [ "$rust_build:winapi" ]
}
if (is_posix) {
main_extern += [ "$rust_build:nix" ]
}

ts_sources = [
"../js/assets.ts",
Expand Down
3 changes: 3 additions & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,6 @@ url = "1.7.2"

[target.'cfg(windows)'.dependencies]
winapi = "0.3.7"

[target.'cfg(unix)'.dependencies]
nix = "0.11.0"
28 changes: 28 additions & 0 deletions cli/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ pub use crate::msg::ErrorKind;
use crate::resolve_addr::ResolveAddrError;
use deno::JSError;
use hyper;
#[cfg(unix)]
use nix::{errno::Errno, Error as UnixError};
use std;
use std::fmt;
use std::io;
Expand Down Expand Up @@ -168,6 +170,32 @@ impl From<ResolveAddrError> for DenoError {
}
}

#[cfg(unix)]
impl From<UnixError> for DenoError {
fn from(e: UnixError) -> Self {
match e {
UnixError::Sys(Errno::EPERM) => Self {
repr: Repr::Simple(
ErrorKind::PermissionDenied,
Errno::EPERM.desc().to_owned(),
),
},
UnixError::Sys(Errno::EINVAL) => Self {
repr: Repr::Simple(
ErrorKind::InvalidInput,
Errno::EINVAL.desc().to_owned(),
),
},
UnixError::Sys(err) => Self {
repr: Repr::Simple(ErrorKind::UnixError, err.desc().to_owned()),
},
_ => Self {
repr: Repr::Simple(ErrorKind::Other, format!("{}", e)),
},
}
}
}

pub fn bad_resource() -> DenoError {
new(ErrorKind::BadResource, String::from("bad resource id"))
}
Expand Down
3 changes: 3 additions & 0 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ extern crate futures;
extern crate serde_json;
extern crate clap;
extern crate deno;
#[cfg(unix)]
extern crate nix;

mod ansi;
pub mod compiler;
Expand All @@ -27,6 +29,7 @@ pub mod permissions;
mod repl;
pub mod resolve_addr;
pub mod resources;
mod signal;
mod startup_data;
pub mod state;
mod tokio_util;
Expand Down
9 changes: 8 additions & 1 deletion cli/msg.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ union Any {
GlobalTimerStop,
IsTTY,
IsTTYRes,
Kill,
Link,
Listen,
ListenRes,
Expand Down Expand Up @@ -129,7 +130,8 @@ enum ErrorKind: byte {
InvalidUri,
InvalidSeekMode,
OpNotAvaiable,
WorkerInitFailed
WorkerInitFailed,
UnixError,
}

table Cwd {}
Expand Down Expand Up @@ -453,6 +455,11 @@ table Close {
rid: uint32;
}

table Kill {
pid: int32;
signo: int32;
}

table Shutdown {
rid: uint32;
how: uint;
Expand Down
17 changes: 17 additions & 0 deletions cli/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::resolve_addr::resolve_addr;
use crate::resources;
use crate::resources::table_entries;
use crate::resources::Resource;
use crate::signal::kill;
use crate::startup_data;
use crate::state::ThreadSafeState;
use crate::tokio_util;
Expand Down Expand Up @@ -171,6 +172,7 @@ pub fn op_selector_std(inner_type: msg::Any) -> Option<OpCreator> {
msg::Any::GlobalTimer => Some(op_global_timer),
msg::Any::GlobalTimerStop => Some(op_global_timer_stop),
msg::Any::IsTTY => Some(op_is_tty),
msg::Any::Kill => Some(op_kill),
msg::Any::Link => Some(op_link),
msg::Any::Listen => Some(op_listen),
msg::Any::MakeTempDir => Some(op_make_temp_dir),
Expand Down Expand Up @@ -906,6 +908,21 @@ fn op_close(
}
}

fn op_kill(
_state: &ThreadSafeState,
base: &msg::Base<'_>,
data: deno_buf,
) -> Box<OpWithError> {
assert_eq!(data.len(), 0);
let inner = base.inner_as_kill().unwrap();
let pid = inner.pid();
let signo = inner.signo();
match kill(pid, signo) {
Ok(_) => ok_future(empty_buf()),
Err(e) => odd_future(e),
}
}

fn op_shutdown(
_state: &ThreadSafeState,
base: &msg::Base<'_>,
Expand Down
20 changes: 20 additions & 0 deletions cli/signal.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#[cfg(unix)]
use nix::sys::signal::{kill as unix_kill, Signal};
#[cfg(unix)]
use nix::unistd::Pid;

use crate::errors::DenoResult;

#[cfg(unix)]
pub fn kill(pid: i32, signo: i32) -> DenoResult<()> {
use crate::errors::DenoError;
let sig = Signal::from_c_int(signo)?;
unix_kill(Pid::from_raw(pid), Option::Some(sig)).map_err(DenoError::from)
}

#[cfg(not(unix))]
pub fn kill(_pid: i32, _signal: i32) -> DenoResult<()> {
// NOOP
// TODO: implement this for windows
Ok(())
}
9 changes: 8 additions & 1 deletion js/deno.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,14 @@ export { FileInfo } from "./file_info";
export { connect, dial, listen, Listener, Conn } from "./net";
export { metrics, Metrics } from "./metrics";
export { resources } from "./resources";
export { run, RunOptions, Process, ProcessStatus } from "./process";
export {
kill,
run,
RunOptions,
Process,
ProcessStatus,
Signal
} from "./process";
export { inspect } from "./console";
export { build, platform, OperatingSystem, Arch } from "./build";
export { version } from "./version";
Expand Down
48 changes: 48 additions & 0 deletions js/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,16 @@ async function runStatus(rid: number): Promise<ProcessStatus> {
}
}

/** Send a signal to process under given PID. Unix only at this moment.
* If pid is negative, the signal will be sent to the process group identified
* by -pid.
*/
export function kill(pid: number, signo: number): void {
const builder = flatbuffers.createBuilder();
const inner = msg.Kill.createKill(builder, pid, signo);
dispatch.sendSync(builder, msg.Any.Kill, inner);
}

export class Process {
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to this commit, but I think we should rename this to Subprocess.

readonly rid: number;
readonly pid: number;
Expand Down Expand Up @@ -113,6 +123,10 @@ export class Process {
close(): void {
close(this.rid);
}

kill(signo: number): void {
kill(this.pid, signo);
}
}

export interface ProcessStatus {
Expand Down Expand Up @@ -179,3 +193,37 @@ export function run(opt: RunOptions): Process {

return new Process(res);
}

export enum Signal {
SIGHUP = 1,
SIGINT,
SIGQUIT,
Copy link
Member

Choose a reason for hiding this comment

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

Although it's equivalent, I would prefer explicitly assigning values to each of these. It just makes things a bit more obvious if it ever needs to be debugged.

SIGINT = 2,
SIGQUIT = 3,
/* .. */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I've just realized that certain signal numbers are platform dependent (like 10 is SIGBUS on macOS but SIGUSR1 on Linux). I might also need to check the current platform to decide which set of signals to export.

Copy link
Member

@ry ry Apr 22, 2019

Choose a reason for hiding this comment

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

Ah too bad. I thought maybe posix defined those - apparently not.
I think there's two options:

  1. Do something similar to Deno.platform where we generate the typescript during build.
  2. Pass the signals as strings ("SIGINT") and map them to their platform dependent integers outside of V8. This is what we did in Node IIRC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently resorting to

export const Signal = platform.os === "mac" ? MacOSSignal : LinuxSignal;

Using string names also not quite ideal unless we build a similar mapping on the Rust side, since from_str in nix crate assumes mostly linux signal name mappings

Copy link
Member

Choose a reason for hiding this comment

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

Ok that works

SIGILL,
SIGTRAP,
SIGABRT,
SIGBUS,
SIGFPE,
SIGKILL,
SIGUSR1,
SIGSEGV,
SIGUSR2,
SIGPIPE,
SIGALRM,
SIGTERM,
SIGSTKFLT,
SIGCHLD,
SIGCONT,
SIGSTOP,
SIGTSTP,
SIGTTIN,
SIGTTOU,
SIGURG,
SIGXCPU,
SIGXFSZ,
SIGVTALRM,
SIGPROF,
SIGWINCH,
SIGIO,
SIGPWR,
SIGSYS
}
60 changes: 59 additions & 1 deletion js/process_test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
import { test, testPerm, assert, assertEquals } from "./test_util.ts";
const { run, DenoError, ErrorKind } = Deno;
const { kill, run, DenoError, ErrorKind } = Deno;

test(function runPermissions(): void {
let caughtError = false;
Expand Down Expand Up @@ -223,3 +223,61 @@ testPerm({ run: true }, async function runEnv(): Promise<void> {
assertEquals(s, "01234567");
p.close();
});

testPerm({ run: true }, async function runClose(): Promise<void> {
const p = run({
args: [
"python",
"-c",
"from time import sleep; import sys; sleep(10000); sys.stderr.write('error')"
],
stderr: "piped"
});
assert(!p.stdin);
assert(!p.stdout);

p.close();

const data = new Uint8Array(10);
let r = await p.stderr.read(data);
assertEquals(r.nread, 0);
assertEquals(r.eof, true);
});

// Ignore signal tests on windows for now...
if (Deno.platform.os !== "win") {
testPerm({ run: true }, async function killSuccess(): Promise<void> {
const p = run({
args: ["python", "-c", "from time import sleep; sleep(10000)"]
});

assertEquals(Deno.Signal.SIGINT, 2);
kill(p.pid, Deno.Signal.SIGINT);
const status = await p.status();

assertEquals(status.success, false);
assertEquals(status.code, undefined);
assertEquals(status.signal, Deno.Signal.SIGINT);
});

testPerm({ run: true }, async function killFailed(): Promise<void> {
const p = run({
args: ["python", "-c", "from time import sleep; sleep(10000)"]
});
assert(!p.stdin);
assert(!p.stdout);

let err;
try {
kill(p.pid, 12345);
} catch (e) {
err = e;
}

assert(!!err);
assertEquals(err.kind, Deno.ErrorKind.InvalidInput);
assertEquals(err.name, "InvalidInput");

p.close();
});
}