Skip to content

Commit 1f576ed

Browse files
committed
Emerald: Fix bug when passing file as stdio
This is not good, we should replace with `dups` or other method to duplicate the file. The issue was, when passing a file, we pass an `fd`, but we still own the file inside `Stdio::Fd`, which will be closed by the parent program, but its already moved to the new process. this causes a crash on close
1 parent ec37380 commit 1f576ed

File tree

1 file changed

+38
-10
lines changed

1 file changed

+38
-10
lines changed

library/std/src/sys/pal/emerald/process.rs

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ pub struct Command {
4242
stdin: Option<Stdio>,
4343
stdout: Option<Stdio>,
4444
stderr: Option<Stdio>,
45+
46+
spawned: bool,
4547
}
4648

4749
// passed back to std::process with the pipes connected to the child, if any
@@ -93,6 +95,8 @@ impl Command {
9395
stdin: None,
9496
stdout: None,
9597
stderr: None,
98+
99+
spawned: false,
96100
}
97101
}
98102

@@ -153,14 +157,20 @@ impl Command {
153157
None
154158
}
155159

156-
fn setup_io(&self, default: Stdio, needs_stdin: bool) -> io::Result<(StdioPipes, ChildPipes)> {
157-
let null = Stdio::Null;
158-
let default_stdin = if needs_stdin { &default } else { &null };
159-
let stdin = self.stdin.as_ref().unwrap_or(default_stdin);
160-
let stdout = self.stdout.as_ref().unwrap_or(&default);
161-
let stderr = self.stderr.as_ref().unwrap_or(&default);
160+
fn setup_io(
161+
&mut self,
162+
mut default: Stdio,
163+
needs_stdin: bool,
164+
) -> io::Result<(StdioPipes, ChildPipes)> {
165+
let mut null = Stdio::Null;
166+
let default_stdin = if needs_stdin { &mut default } else { &mut null };
167+
let stdin = self.stdin.as_mut().unwrap_or(default_stdin);
162168
let (their_stdin, our_stdin) = stdin.to_child_stdio(true)?;
169+
170+
let stdout = self.stdout.as_mut().unwrap_or(&mut default);
163171
let (their_stdout, our_stdout) = stdout.to_child_stdio(false)?;
172+
173+
let stderr = self.stderr.as_mut().unwrap_or(&mut default);
164174
let (their_stderr, our_stderr) = stderr.to_child_stdio(false)?;
165175
let ours = StdioPipes { stdin: our_stdin, stdout: our_stdout, stderr: our_stderr };
166176
let theirs = ChildPipes { stdin: their_stdin, stdout: their_stdout, stderr: their_stderr };
@@ -172,7 +182,16 @@ impl Command {
172182
default: Stdio,
173183
needs_stdin: bool,
174184
) -> io::Result<(Process, StdioPipes)> {
185+
// TODO: add support for multiple spawns.
186+
// this requires `dups` or `clone` for files.
187+
// the reason its like that is that `Stdio::Fd` must be moved
188+
// so we need a way to clone the file
189+
if self.spawned {
190+
return Err(io::Error::new(io::ErrorKind::Other, "Command can only be spawned once"));
191+
}
192+
175193
let (ours, theirs) = self.setup_io(default, needs_stdin)?;
194+
self.spawned = true;
176195

177196
// setup 3 mappings as the max, and only use what's needed
178197
let mut file_mappings = [SpawnFileMapping { src_fd: 0, dst_fd: 0 }; 3];
@@ -212,8 +231,8 @@ impl Command {
212231
}
213232

214233
impl Stdio {
215-
pub fn to_child_stdio(&self, readable: bool) -> io::Result<(ChildStdio, Option<AnonPipe>)> {
216-
match *self {
234+
pub fn to_child_stdio(&mut self, readable: bool) -> io::Result<(ChildStdio, Option<AnonPipe>)> {
235+
match self {
217236
Stdio::Inherit => Ok((ChildStdio::Inherit, None)),
218237

219238
// Make sure that the source descriptors are not an stdio
@@ -223,7 +242,16 @@ impl Stdio {
223242
// parent's stdout, and the child's stdout to be the parent's
224243
// stderr. No matter which we dup first, the second will get
225244
// overwritten prematurely.
226-
Stdio::Fd(ref fd) => {
245+
Stdio::Fd(_) => {
246+
// replace self with inherit
247+
// TODO: this is not good, replace with `dups` or cloning the file somehow
248+
let fd = core::mem::replace(self, Stdio::Inherit);
249+
250+
let fd = match fd {
251+
Stdio::Fd(fd) => fd,
252+
_ => unreachable!(),
253+
};
254+
227255
if fd.as_raw_fd() <= FD_STDERR {
228256
// TODO: add support for passing stdio fds
229257
Err(io::Error::new(
@@ -232,7 +260,7 @@ impl Stdio {
232260
))
233261
} else {
234262
// move the fd
235-
Ok((ChildStdio::Explicit(fd.as_raw_fd()), None))
263+
Ok((ChildStdio::Explicit(fd.into_raw_fd()), None))
236264
}
237265
}
238266

0 commit comments

Comments
 (0)