Skip to content

Commit d13bec7

Browse files
committed
new lint: zombie_processes
1 parent 1156375 commit d13bec7

11 files changed

+232
-3
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5751,6 +5751,7 @@ Released 2018-09-13
57515751
[`zero_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_ptr
57525752
[`zero_sized_map_values`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_sized_map_values
57535753
[`zero_width_space`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_width_space
5754+
[`zombie_processes`]: https://rust-lang.github.io/rust-clippy/master/index.html#zombie_processes
57545755
[`zst_offset`]: https://rust-lang.github.io/rust-clippy/master/index.html#zst_offset
57555756
<!-- end autogenerated links to lint list -->
57565757
<!-- begin autogenerated links to configuration documentation -->

clippy_dev/src/serve.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub fn run(port: u16, lint: Option<&String>) -> ! {
2525
}
2626
if let Some(url) = url.take() {
2727
thread::spawn(move || {
28-
Command::new("python3")
28+
let mut child = Command::new("python3")
2929
.arg("-m")
3030
.arg("http.server")
3131
.arg(port.to_string())
@@ -36,6 +36,7 @@ pub fn run(port: u16, lint: Option<&String>) -> ! {
3636
thread::sleep(Duration::from_millis(500));
3737
// Launch browser after first export.py has completed and http.server is up
3838
let _result = opener::open(url);
39+
child.wait().unwrap();
3940
});
4041
}
4142
thread::sleep(Duration::from_millis(1000));

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -741,4 +741,5 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
741741
crate::write::WRITE_WITH_NEWLINE_INFO,
742742
crate::zero_div_zero::ZERO_DIVIDED_BY_ZERO_INFO,
743743
crate::zero_sized_map_values::ZERO_SIZED_MAP_VALUES_INFO,
744+
crate::zombie_processes::ZOMBIE_PROCESSES_INFO,
744745
];

clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,7 @@ mod wildcard_imports;
367367
mod write;
368368
mod zero_div_zero;
369369
mod zero_sized_map_values;
370+
mod zombie_processes;
370371
// end lints modules, do not remove this comment, it’s used in `update_lints`
371372

372373
use clippy_config::{get_configuration_metadata, Conf};
@@ -1105,6 +1106,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
11051106
});
11061107
store.register_late_pass(move |_| Box::new(incompatible_msrv::IncompatibleMsrv::new(msrv())));
11071108
store.register_late_pass(|_| Box::new(to_string_trait_impl::ToStringTraitImpl));
1109+
store.register_late_pass(|_| Box::new(zombie_processes::ZombieProcesses));
11081110
// add lints here, do not remove this comment, it's used in `new_lint`
11091111
}
11101112

clippy_lints/src/zombie_processes.rs

+114
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::visitors::for_each_local_use_after_expr;
3+
use clippy_utils::{fn_def_id, match_any_def_paths, match_def_path, paths};
4+
use rustc_ast::Mutability;
5+
use rustc_hir::{Expr, ExprKind, Node, PatKind, Stmt, StmtKind};
6+
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_session::{declare_lint_pass, declare_tool_lint};
8+
use rustc_span::Span;
9+
use std::ops::ControlFlow;
10+
11+
declare_clippy_lint! {
12+
/// ### What it does
13+
/// Looks for code that spawns a process but never calls `wait()` on the child.
14+
///
15+
/// ### Why is this bad?
16+
/// As explained in the [standard library documentation](https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning),
17+
/// calling `wait()` is necessary on Unix platforms to properly release all OS resources associated with the process.
18+
/// Not doing so will effectively leak process IDs and/or other limited global resources,
19+
/// which can eventually lead to resource exhaustion, so it's recommended to call `wait()` in long-running applications.
20+
/// Such processes are called "zombie processes".
21+
///
22+
/// ### Example
23+
/// ```rust
24+
/// use std::process::Command;
25+
///
26+
/// let _child = Command::new("ls").spawn().expect("failed to execute child");
27+
/// ```
28+
/// Use instead:
29+
/// ```rust
30+
/// use std::process::Command;
31+
///
32+
/// let mut child = Command::new("ls").spawn().expect("failed to execute child");
33+
/// child.wait().expect("failed to wait on child");
34+
/// ```
35+
#[clippy::version = "1.74.0"]
36+
pub ZOMBIE_PROCESSES,
37+
suspicious,
38+
"not waiting on a spawned child process"
39+
}
40+
declare_lint_pass!(ZombieProcesses => [ZOMBIE_PROCESSES]);
41+
42+
fn emit_lint(cx: &LateContext<'_>, span: Span) {
43+
span_lint_and_then(
44+
cx,
45+
ZOMBIE_PROCESSES,
46+
span,
47+
"spawned process is never `wait()`-ed on and leaves behind a zombie process",
48+
|diag| {
49+
diag.help("consider calling `.wait()`")
50+
.note("also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning");
51+
},
52+
);
53+
}
54+
55+
impl<'tcx> LateLintPass<'tcx> for ZombieProcesses {
56+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
57+
if let ExprKind::Call(..) | ExprKind::MethodCall(..) = expr.kind
58+
&& let Some(child_adt) = cx.typeck_results().expr_ty(expr).ty_adt_def()
59+
&& match_def_path(cx, child_adt.did(), &paths::CHILD)
60+
{
61+
match cx.tcx.hir().get_parent(expr.hir_id) {
62+
Node::Local(local) if let PatKind::Binding(_, local_id, ..) = local.pat.kind => {
63+
64+
// If the `Child` is assigned to a variable, we want to check if the code never calls `.wait()`
65+
// on the variable, and lint if not.
66+
// This is difficult to do because expressions can be arbitrarily complex
67+
// and the variable can "escape" in various ways, e.g. you can take a `&mut` to the variable
68+
// and call `.wait()` through that, or pass it to another function...
69+
// So instead we do the inverse, checking if all uses are either:
70+
// - a field access (`child.{stderr,stdin,stdout}`)
71+
// - calling `id` or `kill`
72+
// - no use at all (e.g. `let _x = child;`)
73+
// - taking a shared reference (`&`), `wait()` can't go through that
74+
// Neither of these is sufficient to prevent zombie processes
75+
// Doing it like this means more FNs, but FNs are better than FPs.
76+
let has_no_wait = for_each_local_use_after_expr(cx, local_id, expr.hir_id, |expr| {
77+
match cx.tcx.hir().get_parent(expr.hir_id) {
78+
Node::Stmt(Stmt { kind: StmtKind::Semi(_), .. }) => ControlFlow::Continue(()),
79+
Node::Expr(expr) if let ExprKind::Field(..) = expr.kind => ControlFlow::Continue(()),
80+
Node::Expr(expr) if let ExprKind::AddrOf(_, Mutability::Not, _) = expr.kind => {
81+
ControlFlow::Continue(())
82+
}
83+
Node::Expr(expr)
84+
if let Some(fn_did) = fn_def_id(cx, expr)
85+
&& match_any_def_paths(cx, fn_did, &[
86+
&paths::CHILD_ID,
87+
&paths::CHILD_KILL,
88+
]).is_some() =>
89+
{
90+
ControlFlow::Continue(())
91+
}
92+
93+
// Conservatively assume that all other kinds of nodes call `.wait()` somehow.
94+
_ => ControlFlow::Break(()),
95+
}
96+
}).is_continue();
97+
98+
if has_no_wait {
99+
emit_lint(cx, expr.span);
100+
}
101+
},
102+
Node::Local(local) if let PatKind::Wild = local.pat.kind => {
103+
// `let _ = child;`, also dropped immediately without `wait()`ing
104+
emit_lint(cx, expr.span);
105+
}
106+
Node::Stmt(Stmt { kind: StmtKind::Semi(_), .. }) => {
107+
// Immediately dropped. E.g. `std::process::Command::new("echo").spawn().unwrap();`
108+
emit_lint(cx, expr.span);
109+
}
110+
_ => {}
111+
}
112+
}
113+
}
114+
}

clippy_utils/src/paths.rs

+4
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ pub const CORE_RESULT_OK_METHOD: [&str; 4] = ["core", "result", "Result", "ok"];
2424
pub const CSTRING_AS_C_STR: [&str; 5] = ["alloc", "ffi", "c_str", "CString", "as_c_str"];
2525
pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"];
2626
pub const EARLY_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "EarlyLintPass"];
27+
pub const EXIT: [&str; 3] = ["std", "process", "exit"];
28+
pub const CHILD: [&str; 3] = ["std", "process", "Child"];
29+
pub const CHILD_ID: [&str; 4] = ["std", "process", "Child", "id"];
30+
pub const CHILD_KILL: [&str; 4] = ["std", "process", "Child", "kill"];
2731
pub const F32_EPSILON: [&str; 4] = ["core", "f32", "<impl f32>", "EPSILON"];
2832
pub const F64_EPSILON: [&str; 4] = ["core", "f64", "<impl f64>", "EPSILON"];
2933
pub const FILE_OPTIONS: [&str; 4] = ["std", "fs", "File", "options"];

tests/ui/suspicious_command_arg_space.fixed

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![allow(clippy::zombie_processes)]
12
fn main() {
23
// Things it should warn about:
34
std::process::Command::new("echo").args(["-n", "hello"]).spawn().unwrap();

tests/ui/suspicious_command_arg_space.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![allow(clippy::zombie_processes)]
12
fn main() {
23
// Things it should warn about:
34
std::process::Command::new("echo").arg("-n hello").spawn().unwrap();

tests/ui/suspicious_command_arg_space.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: single argument that looks like it should be multiple arguments
2-
--> $DIR/suspicious_command_arg_space.rs:3:44
2+
--> $DIR/suspicious_command_arg_space.rs:4:44
33
|
44
LL | std::process::Command::new("echo").arg("-n hello").spawn().unwrap();
55
| ^^^^^^^^^^
@@ -12,7 +12,7 @@ LL | std::process::Command::new("echo").args(["-n", "hello"]).spawn().unwrap
1212
| ~~~~ ~~~~~~~~~~~~~~~
1313

1414
error: single argument that looks like it should be multiple arguments
15-
--> $DIR/suspicious_command_arg_space.rs:6:43
15+
--> $DIR/suspicious_command_arg_space.rs:7:43
1616
|
1717
LL | std::process::Command::new("cat").arg("--number file").spawn().unwrap();
1818
| ^^^^^^^^^^^^^^^

tests/ui/zombie_processes.rs

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
#![warn(clippy::zombie_processes)]
2+
3+
use std::process::{Child, Command};
4+
5+
fn main() {
6+
let _ = Command::new("").spawn().unwrap();
7+
//~^ ERROR: spawned process is never `wait()`-ed on
8+
Command::new("").spawn().unwrap();
9+
//~^ ERROR: spawned process is never `wait()`-ed on
10+
spawn_proc();
11+
//~^ ERROR: spawned process is never `wait()`-ed on
12+
spawn_proc().wait().unwrap(); // OK
13+
14+
{
15+
let mut x = Command::new("").spawn().unwrap();
16+
//~^ ERROR: spawned process is never `wait()`-ed on
17+
x.kill();
18+
x.id();
19+
}
20+
{
21+
let mut x = Command::new("").spawn().unwrap();
22+
x.wait().unwrap(); // OK
23+
}
24+
{
25+
let x = Command::new("").spawn().unwrap();
26+
x.wait_with_output().unwrap(); // OK
27+
}
28+
{
29+
let mut x = Command::new("").spawn().unwrap();
30+
x.try_wait().unwrap(); // OK
31+
}
32+
{
33+
let mut x = Command::new("").spawn().unwrap();
34+
let mut r = &mut x;
35+
r.wait().unwrap(); // OK, not calling `.wait()` directly on `x` but through `r` -> `x`
36+
}
37+
{
38+
let mut x = Command::new("").spawn().unwrap();
39+
process_child(x); // OK, other function might call `.wait()` so assume it does
40+
}
41+
{
42+
let mut x = Command::new("").spawn().unwrap();
43+
//~^ ERROR: spawned process is never `wait()`-ed on
44+
let v = &x;
45+
// (allow shared refs is fine because one cannot call `.wait()` through that)
46+
}
47+
}
48+
49+
fn spawn_proc() -> Child {
50+
todo!()
51+
}
52+
53+
fn process_child(c: Child) {
54+
todo!()
55+
}

tests/ui/zombie_processes.stderr

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
2+
--> $DIR/zombie_processes.rs:6:13
3+
|
4+
LL | let _ = Command::new("").spawn().unwrap();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= help: consider calling `.wait()`
8+
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
9+
= note: `-D clippy::zombie-processes` implied by `-D warnings`
10+
= help: to override `-D warnings` add `#[allow(clippy::zombie_processes)]`
11+
12+
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
13+
--> $DIR/zombie_processes.rs:8:5
14+
|
15+
LL | Command::new("").spawn().unwrap();
16+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
17+
|
18+
= help: consider calling `.wait()`
19+
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
20+
21+
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
22+
--> $DIR/zombie_processes.rs:10:5
23+
|
24+
LL | spawn_proc();
25+
| ^^^^^^^^^^^^
26+
|
27+
= help: consider calling `.wait()`
28+
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
29+
30+
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
31+
--> $DIR/zombie_processes.rs:15:21
32+
|
33+
LL | let mut x = Command::new("").spawn().unwrap();
34+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
35+
|
36+
= help: consider calling `.wait()`
37+
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
38+
39+
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
40+
--> $DIR/zombie_processes.rs:42:21
41+
|
42+
LL | let mut x = Command::new("").spawn().unwrap();
43+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
44+
|
45+
= help: consider calling `.wait()`
46+
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
47+
48+
error: aborting due to 5 previous errors
49+

0 commit comments

Comments
 (0)