native: Fix a possible deadlock in spawn
The OSX bots have been deadlocking recently in the rustdoc tests. I have only been able to rarely reproduce the deadlock on my local setup. When reproduced, it looks like the child process is spinning on the malloc mutex, which I presume is locked with no other threads to unlock it. I'm not convinced that this is what's happening, because OSX should protect against this with pthread_atfork by default. Regardless, running as little code as possible in the child after fork() is normally a good idea anyway, so this commit moves all allocation to the parent process to run before the child executes. After running 6k iterations of rustdoc tests, this deadlocked twice before, and after 20k iterations afterwards, it never deadlocked. I draw the conclusion that this is either sweeping the bug under the rug, or it did indeed fix the underlying problem.
This commit is contained in:
parent
739f22f611
commit
4fdff3e44e
|
@ -446,132 +446,129 @@ fn spawn_process_os(config: p::ProcessConfig,
|
||||||
assert_eq!(ret, 0);
|
assert_eq!(ret, 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
let pipe = os::pipe();
|
let dirp = dir.map(|p| p.to_c_str());
|
||||||
let mut input = file::FileDesc::new(pipe.input, true);
|
let dirp = dirp.as_ref().map(|c| c.with_ref(|p| p)).unwrap_or(ptr::null());
|
||||||
let mut output = file::FileDesc::new(pipe.out, true);
|
|
||||||
|
|
||||||
unsafe { set_cloexec(output.fd()) };
|
with_envp(env, proc(envp) {
|
||||||
|
with_argv(config.program, config.args, proc(argv) unsafe {
|
||||||
|
let pipe = os::pipe();
|
||||||
|
let mut input = file::FileDesc::new(pipe.input, true);
|
||||||
|
let mut output = file::FileDesc::new(pipe.out, true);
|
||||||
|
|
||||||
unsafe {
|
set_cloexec(output.fd());
|
||||||
let pid = fork();
|
|
||||||
if pid < 0 {
|
|
||||||
fail!("failure in fork: {}", os::last_os_error());
|
|
||||||
} else if pid > 0 {
|
|
||||||
drop(output);
|
|
||||||
let mut bytes = [0, ..4];
|
|
||||||
return match input.inner_read(bytes) {
|
|
||||||
Ok(4) => {
|
|
||||||
let errno = (bytes[0] << 24) as i32 |
|
|
||||||
(bytes[1] << 16) as i32 |
|
|
||||||
(bytes[2] << 8) as i32 |
|
|
||||||
(bytes[3] << 0) as i32;
|
|
||||||
Err(super::translate_error(errno, false))
|
|
||||||
}
|
|
||||||
Err(e) => {
|
|
||||||
assert!(e.kind == io::BrokenPipe ||
|
|
||||||
e.kind == io::EndOfFile,
|
|
||||||
"unexpected error: {}", e);
|
|
||||||
Ok(SpawnProcessResult {
|
|
||||||
pid: pid,
|
|
||||||
handle: ptr::null()
|
|
||||||
})
|
|
||||||
}
|
|
||||||
Ok(..) => fail!("short read on the cloexec pipe"),
|
|
||||||
};
|
|
||||||
}
|
|
||||||
drop(input);
|
|
||||||
|
|
||||||
fn fail(output: &mut file::FileDesc) -> ! {
|
let pid = fork();
|
||||||
let errno = os::errno();
|
if pid < 0 {
|
||||||
let bytes = [
|
fail!("failure in fork: {}", os::last_os_error());
|
||||||
(errno << 24) as u8,
|
} else if pid > 0 {
|
||||||
(errno << 16) as u8,
|
drop(output);
|
||||||
(errno << 8) as u8,
|
let mut bytes = [0, ..4];
|
||||||
(errno << 0) as u8,
|
return match input.inner_read(bytes) {
|
||||||
];
|
Ok(4) => {
|
||||||
assert!(output.inner_write(bytes).is_ok());
|
let errno = (bytes[0] << 24) as i32 |
|
||||||
unsafe { libc::_exit(1) }
|
(bytes[1] << 16) as i32 |
|
||||||
}
|
(bytes[2] << 8) as i32 |
|
||||||
|
(bytes[3] << 0) as i32;
|
||||||
rustrt::rust_unset_sigprocmask();
|
Err(super::translate_error(errno, false))
|
||||||
|
}
|
||||||
if in_fd == -1 {
|
Err(e) => {
|
||||||
let _ = libc::close(libc::STDIN_FILENO);
|
assert!(e.kind == io::BrokenPipe ||
|
||||||
} else if retry(|| dup2(in_fd, 0)) == -1 {
|
e.kind == io::EndOfFile,
|
||||||
fail(&mut output);
|
"unexpected error: {}", e);
|
||||||
}
|
Ok(SpawnProcessResult {
|
||||||
if out_fd == -1 {
|
pid: pid,
|
||||||
let _ = libc::close(libc::STDOUT_FILENO);
|
handle: ptr::null()
|
||||||
} else if retry(|| dup2(out_fd, 1)) == -1 {
|
})
|
||||||
fail(&mut output);
|
}
|
||||||
}
|
Ok(..) => fail!("short read on the cloexec pipe"),
|
||||||
if err_fd == -1 {
|
};
|
||||||
let _ = libc::close(libc::STDERR_FILENO);
|
|
||||||
} else if retry(|| dup2(err_fd, 2)) == -1 {
|
|
||||||
fail(&mut output);
|
|
||||||
}
|
|
||||||
// close all other fds
|
|
||||||
for fd in range(3, getdtablesize()).rev() {
|
|
||||||
if fd != output.fd() {
|
|
||||||
let _ = close(fd as c_int);
|
|
||||||
}
|
}
|
||||||
}
|
drop(input);
|
||||||
|
|
||||||
match config.gid {
|
fn fail(output: &mut file::FileDesc) -> ! {
|
||||||
Some(u) => {
|
let errno = os::errno();
|
||||||
if libc::setgid(u as libc::gid_t) != 0 {
|
let bytes = [
|
||||||
fail(&mut output);
|
(errno << 24) as u8,
|
||||||
|
(errno << 16) as u8,
|
||||||
|
(errno << 8) as u8,
|
||||||
|
(errno << 0) as u8,
|
||||||
|
];
|
||||||
|
assert!(output.inner_write(bytes).is_ok());
|
||||||
|
unsafe { libc::_exit(1) }
|
||||||
|
}
|
||||||
|
|
||||||
|
rustrt::rust_unset_sigprocmask();
|
||||||
|
|
||||||
|
if in_fd == -1 {
|
||||||
|
let _ = libc::close(libc::STDIN_FILENO);
|
||||||
|
} else if retry(|| dup2(in_fd, 0)) == -1 {
|
||||||
|
fail(&mut output);
|
||||||
|
}
|
||||||
|
if out_fd == -1 {
|
||||||
|
let _ = libc::close(libc::STDOUT_FILENO);
|
||||||
|
} else if retry(|| dup2(out_fd, 1)) == -1 {
|
||||||
|
fail(&mut output);
|
||||||
|
}
|
||||||
|
if err_fd == -1 {
|
||||||
|
let _ = libc::close(libc::STDERR_FILENO);
|
||||||
|
} else if retry(|| dup2(err_fd, 2)) == -1 {
|
||||||
|
fail(&mut output);
|
||||||
|
}
|
||||||
|
// close all other fds
|
||||||
|
for fd in range(3, getdtablesize()).rev() {
|
||||||
|
if fd != output.fd() {
|
||||||
|
let _ = close(fd as c_int);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
None => {}
|
|
||||||
}
|
|
||||||
match config.uid {
|
|
||||||
Some(u) => {
|
|
||||||
// When dropping privileges from root, the `setgroups` call will
|
|
||||||
// remove any extraneous groups. If we don't call this, then
|
|
||||||
// even though our uid has dropped, we may still have groups
|
|
||||||
// that enable us to do super-user things. This will fail if we
|
|
||||||
// aren't root, so don't bother checking the return value, this
|
|
||||||
// is just done as an optimistic privilege dropping function.
|
|
||||||
extern {
|
|
||||||
fn setgroups(ngroups: libc::c_int,
|
|
||||||
ptr: *libc::c_void) -> libc::c_int;
|
|
||||||
}
|
|
||||||
let _ = setgroups(0, 0 as *libc::c_void);
|
|
||||||
|
|
||||||
if libc::setuid(u as libc::uid_t) != 0 {
|
match config.gid {
|
||||||
fail(&mut output);
|
Some(u) => {
|
||||||
|
if libc::setgid(u as libc::gid_t) != 0 {
|
||||||
|
fail(&mut output);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
None => {}
|
||||||
}
|
}
|
||||||
None => {}
|
match config.uid {
|
||||||
}
|
Some(u) => {
|
||||||
if config.detach {
|
// When dropping privileges from root, the `setgroups` call will
|
||||||
// Don't check the error of setsid because it fails if we're the
|
// remove any extraneous groups. If we don't call this, then
|
||||||
// process leader already. We just forked so it shouldn't return
|
// even though our uid has dropped, we may still have groups
|
||||||
// error, but ignore it anyway.
|
// that enable us to do super-user things. This will fail if we
|
||||||
let _ = libc::setsid();
|
// aren't root, so don't bother checking the return value, this
|
||||||
}
|
// is just done as an optimistic privilege dropping function.
|
||||||
|
extern {
|
||||||
|
fn setgroups(ngroups: libc::c_int,
|
||||||
|
ptr: *libc::c_void) -> libc::c_int;
|
||||||
|
}
|
||||||
|
let _ = setgroups(0, 0 as *libc::c_void);
|
||||||
|
|
||||||
with_dirp(dir, |dirp| {
|
if libc::setuid(u as libc::uid_t) != 0 {
|
||||||
|
fail(&mut output);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
None => {}
|
||||||
|
}
|
||||||
|
if config.detach {
|
||||||
|
// Don't check the error of setsid because it fails if we're the
|
||||||
|
// process leader already. We just forked so it shouldn't return
|
||||||
|
// error, but ignore it anyway.
|
||||||
|
let _ = libc::setsid();
|
||||||
|
}
|
||||||
if !dirp.is_null() && chdir(dirp) == -1 {
|
if !dirp.is_null() && chdir(dirp) == -1 {
|
||||||
fail(&mut output);
|
fail(&mut output);
|
||||||
}
|
}
|
||||||
});
|
|
||||||
|
|
||||||
with_envp(env, |envp| {
|
|
||||||
if !envp.is_null() {
|
if !envp.is_null() {
|
||||||
set_environ(envp);
|
set_environ(envp);
|
||||||
}
|
}
|
||||||
with_argv(config.program, config.args, |argv| {
|
let _ = execvp(*argv, argv);
|
||||||
let _ = execvp(*argv, argv);
|
fail(&mut output);
|
||||||
fail(&mut output);
|
|
||||||
})
|
|
||||||
})
|
})
|
||||||
}
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(unix)]
|
#[cfg(unix)]
|
||||||
fn with_argv<T>(prog: &str, args: &[~str], cb: |**libc::c_char| -> T) -> T {
|
fn with_argv<T>(prog: &str, args: &[~str], cb: proc:(**libc::c_char) -> T) -> T {
|
||||||
use std::slice;
|
use std::slice;
|
||||||
|
|
||||||
// We can't directly convert `str`s into `*char`s, as someone needs to hold
|
// We can't directly convert `str`s into `*char`s, as someone needs to hold
|
||||||
|
@ -597,7 +594,7 @@ fn with_argv<T>(prog: &str, args: &[~str], cb: |**libc::c_char| -> T) -> T {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(unix)]
|
#[cfg(unix)]
|
||||||
fn with_envp<T>(env: Option<~[(~str, ~str)]>, cb: |*c_void| -> T) -> T {
|
fn with_envp<T>(env: Option<~[(~str, ~str)]>, cb: proc:(*c_void) -> T) -> T {
|
||||||
use std::slice;
|
use std::slice;
|
||||||
|
|
||||||
// On posixy systems we can pass a char** for envp, which is a
|
// On posixy systems we can pass a char** for envp, which is a
|
||||||
|
@ -645,6 +642,7 @@ fn with_envp<T>(env: Option<~[(~str, ~str)]>, cb: |*mut c_void| -> T) -> T {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
fn with_dirp<T>(d: Option<&Path>, cb: |*libc::c_char| -> T) -> T {
|
fn with_dirp<T>(d: Option<&Path>, cb: |*libc::c_char| -> T) -> T {
|
||||||
match d {
|
match d {
|
||||||
Some(dir) => dir.with_c_str(|buf| cb(buf)),
|
Some(dir) => dir.with_c_str(|buf| cb(buf)),
|
||||||
|
|
Loading…
Reference in New Issue