From 674d24e2e6fad26fc4cedc2fe935ade63198ba7e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 3 Jan 2014 23:49:56 -0800 Subject: [PATCH] Handle EINTR throughout libnative Closes #11214 --- src/libnative/io/file.rs | 100 +++++++++++++++++++++--------------- src/libnative/io/mod.rs | 17 ++++-- src/libnative/io/net.rs | 44 +++++++++------- src/libnative/io/process.rs | 17 +++--- 4 files changed, 108 insertions(+), 70 deletions(-) diff --git a/src/libnative/io/file.rs b/src/libnative/io/file.rs index 6197bd70c7610..0021dfcb881a4 100644 --- a/src/libnative/io/file.rs +++ b/src/libnative/io/file.rs @@ -20,7 +20,7 @@ use std::rt::rtio; use std::unstable::intrinsics; use std::vec; -use super::IoResult; +use io::{IoResult, retry}; #[cfg(windows)] use std::os::win32::{as_utf16_p, fill_utf16_buf_and_decode}; #[cfg(windows)] use std::ptr; @@ -143,8 +143,8 @@ impl rtio::RtioFileStream for FileDesc { overlap.OffsetHigh = (offset >> 32) as libc::DWORD; match libc::ReadFile(handle, buf as libc::LPVOID, - amt as libc::DWORD, - &mut bytes_read, &mut overlap) { + amt as libc::DWORD, + &mut bytes_read, &mut overlap) { 0 => Err(super::last_error()), _ => Ok(bytes_read as int) } @@ -153,10 +153,10 @@ impl rtio::RtioFileStream for FileDesc { #[cfg(unix)] fn os_pread(fd: c_int, buf: *u8, amt: uint, offset: u64) -> IoResult { - match unsafe { + match retry(|| unsafe { libc::pread(fd, buf as *libc::c_void, amt as libc::size_t, - offset as libc::off_t) - } { + offset as libc::off_t) as libc::c_int + }) { -1 => Err(super::last_error()), n => Ok(n as int) } @@ -184,10 +184,10 @@ impl rtio::RtioFileStream for FileDesc { #[cfg(unix)] fn os_pwrite(fd: c_int, buf: *u8, amt: uint, offset: u64) -> IoResult<()> { - super::mkerr_libc(unsafe { + super::mkerr_libc(retry(|| unsafe { libc::pwrite(fd, buf as *libc::c_void, amt as libc::size_t, offset as libc::off_t) - } as c_int) + } as c_int)) } } #[cfg(windows)] @@ -240,7 +240,7 @@ impl rtio::RtioFileStream for FileDesc { } #[cfg(unix)] fn os_fsync(fd: c_int) -> IoResult<()> { - super::mkerr_libc(unsafe { libc::fsync(fd) }) + super::mkerr_libc(retry(|| unsafe { libc::fsync(fd) })) } } #[cfg(windows)] @@ -255,9 +255,13 @@ impl rtio::RtioFileStream for FileDesc { unsafe { libc::fcntl(fd, libc::F_FULLFSYNC) } } #[cfg(target_os = "linux")] - fn os_datasync(fd: c_int) -> c_int { unsafe { libc::fdatasync(fd) } } + fn os_datasync(fd: c_int) -> c_int { + retry(|| unsafe { libc::fdatasync(fd) }) + } #[cfg(not(target_os = "macos"), not(target_os = "linux"))] - fn os_datasync(fd: c_int) -> c_int { unsafe { libc::fsync(fd) } } + fn os_datasync(fd: c_int) -> c_int { + retry(|| unsafe { libc::fsync(fd) }) + } } #[cfg(windows)] @@ -278,9 +282,9 @@ impl rtio::RtioFileStream for FileDesc { } #[cfg(unix)] fn truncate(&mut self, offset: i64) -> Result<(), IoError> { - super::mkerr_libc(unsafe { + super::mkerr_libc(retry(|| unsafe { libc::ftruncate(self.fd, offset as libc::off_t) - }) + })) } } @@ -311,9 +315,17 @@ impl rtio::RtioTTY for FileDesc { impl Drop for FileDesc { fn drop(&mut self) { - // closing stdio file handles makes no sense, so never do it + // closing stdio file handles makes no sense, so never do it. Also, note + // that errors are ignored when closing a file descriptor. The reason + // for this is that if an error occurs we don't actually know if the + // file descriptor was closed or not, and if we retried (for something + // like EINTR), we might close another valid file descriptor (opened + // after we closed ours. if self.close_on_drop && self.fd > libc::STDERR_FILENO { - unsafe { libc::close(self.fd); } + let n = unsafe { libc::close(self.fd) }; + if n != 0 { + warn!("error {} when closing file descriptor {}", n, self.fd); + } } } } @@ -336,7 +348,7 @@ impl CFile { } pub fn flush(&mut self) -> Result<(), IoError> { - super::mkerr_libc(unsafe { libc::fflush(self.file) }) + super::mkerr_libc(retry(|| unsafe { libc::fflush(self.file) })) } } @@ -444,13 +456,13 @@ pub fn open(path: &CString, fm: io::FileMode, fa: io::FileAccess) #[cfg(windows)] fn os_open(path: &CString, flags: c_int, mode: c_int) -> c_int { as_utf16_p(path.as_str().unwrap(), |path| { - unsafe { libc::wopen(path, flags, mode) } + retry(|| unsafe { libc::wopen(path, flags, mode) }) }) } #[cfg(unix)] fn os_open(path: &CString, flags: c_int, mode: c_int) -> c_int { - unsafe { libc::open(path.with_ref(|p| p), flags, mode) } + retry(|| unsafe { libc::open(path.with_ref(|p| p), flags, mode) }) } } @@ -469,9 +481,9 @@ pub fn mkdir(p: &CString, mode: io::FilePermission) -> IoResult<()> { #[cfg(unix)] fn os_mkdir(p: &CString, mode: c_int) -> IoResult<()> { - super::mkerr_libc(unsafe { + super::mkerr_libc(retry(|| unsafe { libc::mkdir(p.with_ref(|p| p), mode as libc::mode_t) - }) + })) } } @@ -582,7 +594,7 @@ pub fn unlink(p: &CString) -> IoResult<()> { #[cfg(unix)] fn os_unlink(p: &CString) -> IoResult<()> { - super::mkerr_libc(unsafe { libc::unlink(p.with_ref(|p| p)) }) + super::mkerr_libc(retry(|| unsafe { libc::unlink(p.with_ref(|p| p)) })) } } @@ -602,9 +614,9 @@ pub fn rename(old: &CString, new: &CString) -> IoResult<()> { #[cfg(unix)] fn os_rename(old: &CString, new: &CString) -> IoResult<()> { - super::mkerr_libc(unsafe { + super::mkerr_libc(retry(|| unsafe { libc::rename(old.with_ref(|p| p), new.with_ref(|p| p)) - }) + })) } } @@ -614,13 +626,15 @@ pub fn chmod(p: &CString, mode: io::FilePermission) -> IoResult<()> { #[cfg(windows)] fn os_chmod(p: &CString, mode: c_int) -> c_int { unsafe { - as_utf16_p(p.as_str().unwrap(), |p| libc::wchmod(p, mode)) + as_utf16_p(p.as_str().unwrap(), |p| retry(|| { + libc::wchmod(p, mode) + })) } } #[cfg(unix)] fn os_chmod(p: &CString, mode: c_int) -> c_int { - unsafe { libc::chmod(p.with_ref(|p| p), mode as libc::mode_t) } + retry(||unsafe { libc::chmod(p.with_ref(|p| p), mode as libc::mode_t) }) } } @@ -630,13 +644,15 @@ pub fn rmdir(p: &CString) -> IoResult<()> { #[cfg(windows)] fn os_rmdir(p: &CString) -> c_int { unsafe { - as_utf16_p(p.as_str().unwrap(), |p| libc::wrmdir(p)) + as_utf16_p(p.as_str().unwrap(), |p| retry(|| { + libc::wrmdir(p) + })) } } #[cfg(unix)] fn os_rmdir(p: &CString) -> c_int { - unsafe { libc::rmdir(p.with_ref(|p| p)) } + retry(|| unsafe { libc::rmdir(p.with_ref(|p| p)) }) } } @@ -649,10 +665,10 @@ pub fn chown(p: &CString, uid: int, gid: int) -> IoResult<()> { #[cfg(unix)] fn os_chown(p: &CString, uid: int, gid: int) -> c_int { - unsafe { + retry(|| unsafe { libc::chown(p.with_ref(|p| p), uid as libc::uid_t, gid as libc::gid_t) - } + }) } } @@ -697,10 +713,10 @@ pub fn readlink(p: &CString) -> IoResult { len = 1024; // XXX: read PATH_MAX from C ffi? } let mut buf = vec::with_capacity::(len as uint); - match unsafe { + match retry(|| unsafe { libc::readlink(p, buf.as_ptr() as *mut libc::c_char, - len as libc::size_t) - } { + len as libc::size_t) as libc::c_int + }) { -1 => Err(super::last_error()), n => { assert!(n > 0); @@ -725,9 +741,9 @@ pub fn symlink(src: &CString, dst: &CString) -> IoResult<()> { #[cfg(unix)] fn os_symlink(src: &CString, dst: &CString) -> IoResult<()> { - super::mkerr_libc(unsafe { + super::mkerr_libc(retry(|| unsafe { libc::symlink(src.with_ref(|p| p), dst.with_ref(|p| p)) - }) + })) } } @@ -745,9 +761,9 @@ pub fn link(src: &CString, dst: &CString) -> IoResult<()> { #[cfg(unix)] fn os_link(src: &CString, dst: &CString) -> IoResult<()> { - super::mkerr_libc(unsafe { + super::mkerr_libc(retry(|| unsafe { libc::link(src.with_ref(|p| p), dst.with_ref(|p| p)) - }) + })) } } @@ -842,7 +858,7 @@ pub fn stat(p: &CString) -> IoResult { fn os_stat(p: &CString) -> IoResult { let mut stat: libc::stat = unsafe { intrinsics::uninit() }; as_utf16_p(p.as_str().unwrap(), |up| { - match unsafe { libc::wstat(up, &mut stat) } { + match retry(|| unsafe { libc::wstat(up, &mut stat) }) { 0 => Ok(mkstat(&stat, p)), _ => Err(super::last_error()), } @@ -852,7 +868,7 @@ pub fn stat(p: &CString) -> IoResult { #[cfg(unix)] fn os_stat(p: &CString) -> IoResult { let mut stat: libc::stat = unsafe { intrinsics::uninit() }; - match unsafe { libc::stat(p.with_ref(|p| p), &mut stat) } { + match retry(|| unsafe { libc::stat(p.with_ref(|p| p), &mut stat) }) { 0 => Ok(mkstat(&stat, p)), _ => Err(super::last_error()), } @@ -871,7 +887,7 @@ pub fn lstat(p: &CString) -> IoResult { #[cfg(unix)] fn os_lstat(p: &CString) -> IoResult { let mut stat: libc::stat = unsafe { intrinsics::uninit() }; - match unsafe { libc::lstat(p.with_ref(|p| p), &mut stat) } { + match retry(|| unsafe { libc::lstat(p.with_ref(|p| p), &mut stat) }) { 0 => Ok(mkstat(&stat, p)), _ => Err(super::last_error()), } @@ -888,7 +904,9 @@ pub fn utime(p: &CString, atime: u64, mtime: u64) -> IoResult<()> { modtime: (mtime / 1000) as libc::time64_t, }; unsafe { - as_utf16_p(p.as_str().unwrap(), |p| libc::wutime(p, &buf)) + as_utf16_p(p.as_str().unwrap(), |p| retry(|| { + libc::wutime(p, &buf) + })) } } @@ -898,7 +916,7 @@ pub fn utime(p: &CString, atime: u64, mtime: u64) -> IoResult<()> { actime: (atime / 1000) as libc::time_t, modtime: (mtime / 1000) as libc::time_t, }; - unsafe { libc::utime(p.with_ref(|p| p), &buf) } + retry(|| unsafe { libc::utime(p.with_ref(|p| p), &buf) }) } } diff --git a/src/libnative/io/mod.rs b/src/libnative/io/mod.rs index b936a36cf3a65..f1bec440547e1 100644 --- a/src/libnative/io/mod.rs +++ b/src/libnative/io/mod.rs @@ -134,13 +134,24 @@ fn mkerr_winbool(ret: libc::c_int) -> IoResult<()> { } } +#[cfg(windows)] +#[inline] +fn retry(f: || -> libc::c_int) -> libc::c_int { + loop { + match f() { + -1 if os::errno() as int == libc::WSAEINTR as int => {} + n => return n, + } + } +} + #[cfg(unix)] -fn retry(f: || -> libc::c_int) -> IoResult { +#[inline] +fn retry(f: || -> libc::c_int) -> libc::c_int { loop { match f() { -1 if os::errno() as int == libc::EINTR as int => {} - -1 => return Err(last_error()), - n => return Ok(n), + n => return n, } } } diff --git a/src/libnative/io/net.rs b/src/libnative/io/net.rs index 241a69ad4e6f1..b26ac141192bf 100644 --- a/src/libnative/io/net.rs +++ b/src/libnative/io/net.rs @@ -16,7 +16,7 @@ use std::mem; use std::rt::rtio; use std::unstable::intrinsics; -use super::IoResult; +use super::{IoResult, retry}; use super::file::keep_going; //////////////////////////////////////////////////////////////////////////////// @@ -227,8 +227,10 @@ impl TcpStream { let (addr, len) = addr_to_sockaddr(addr); let addrp = &addr as *libc::sockaddr_storage; let ret = TcpStream { fd: fd }; - match libc::connect(fd, addrp as *libc::sockaddr, - len as libc::socklen_t) { + match retry(|| { + libc::connect(fd, addrp as *libc::sockaddr, + len as libc::socklen_t) + }) { -1 => Err(super::last_error()), _ => Ok(ret), } @@ -394,9 +396,11 @@ impl TcpAcceptor { let storagep = &mut storage as *mut libc::sockaddr_storage; let size = mem::size_of::(); let mut size = size as libc::socklen_t; - match libc::accept(self.fd(), - storagep as *mut libc::sockaddr, - &mut size as *mut libc::socklen_t) { + match retry(|| { + libc::accept(self.fd(), + storagep as *mut libc::sockaddr, + &mut size as *mut libc::socklen_t) as libc::c_int + }) as sock_t { -1 => Err(super::last_error()), fd => Ok(TcpStream { fd: fd }) } @@ -493,12 +497,14 @@ impl rtio::RtioUdpSocket for UdpSocket { let storagep = &mut storage as *mut libc::sockaddr_storage; let mut addrlen: libc::socklen_t = mem::size_of::() as libc::socklen_t; - let ret = libc::recvfrom(self.fd, - buf.as_ptr() as *mut libc::c_void, - buf.len() as msglen_t, - 0, - storagep as *mut libc::sockaddr, - &mut addrlen); + let ret = retry(|| { + libc::recvfrom(self.fd, + buf.as_ptr() as *mut libc::c_void, + buf.len() as msglen_t, + 0, + storagep as *mut libc::sockaddr, + &mut addrlen) as libc::c_int + }); if ret < 0 { return Err(super::last_error()) } sockaddr_to_addr(&storage, addrlen as uint).and_then(|addr| { Ok((ret as uint, addr)) @@ -509,12 +515,14 @@ impl rtio::RtioUdpSocket for UdpSocket { let (dst, len) = addr_to_sockaddr(dst); let dstp = &dst as *libc::sockaddr_storage; unsafe { - let ret = libc::sendto(self.fd, - buf.as_ptr() as *libc::c_void, - buf.len() as msglen_t, - 0, - dstp as *libc::sockaddr, - len as libc::socklen_t); + let ret = retry(|| { + libc::sendto(self.fd, + buf.as_ptr() as *libc::c_void, + buf.len() as msglen_t, + 0, + dstp as *libc::sockaddr, + len as libc::socklen_t) as libc::c_int + }); match ret { -1 => Err(super::last_error()), n if n as uint != buf.len() => { diff --git a/src/libnative/io/process.rs b/src/libnative/io/process.rs index 3fda448692159..0569c45f6def9 100644 --- a/src/libnative/io/process.rs +++ b/src/libnative/io/process.rs @@ -16,11 +16,12 @@ use std::ptr; use std::rt::rtio; use p = std::io::process; -#[cfg(windows)] use std::cast; - use super::IoResult; use super::file; +#[cfg(windows)] use std::cast; +#[cfg(not(windows))] use super::retry; + /** * A value representing a child process. * @@ -445,17 +446,17 @@ fn spawn_process_os(prog: &str, args: &[~str], if in_fd == -1 { libc::close(libc::STDIN_FILENO); - } else if dup2(in_fd, 0) == -1 { + } else if retry(|| dup2(in_fd, 0)) == -1 { fail!("failure in dup2(in_fd, 0): {}", os::last_os_error()); } if out_fd == -1 { libc::close(libc::STDOUT_FILENO); - } else if dup2(out_fd, 1) == -1 { + } else if retry(|| dup2(out_fd, 1)) == -1 { fail!("failure in dup2(out_fd, 1): {}", os::last_os_error()); } if err_fd == -1 { libc::close(libc::STDERR_FILENO); - } else if dup2(err_fd, 2) == -1 { + } else if retry(|| dup2(err_fd, 2)) == -1 { fail!("failure in dup3(err_fd, 2): {}", os::last_os_error()); } // close all other fds @@ -664,9 +665,9 @@ fn waitpid(pid: pid_t) -> p::ProcessExit { } let mut status = 0 as c_int; - match super::retry(|| unsafe { wait::waitpid(pid, &mut status, 0) }) { - Err(e) => fail!("unknown waitpid error: {:?}", e), - Ok(_ret) => { + match retry(|| unsafe { wait::waitpid(pid, &mut status, 0) }) { + -1 => fail!("unknown waitpid error: {:?}", super::last_error()), + _ => { if imp::WIFEXITED(status) { p::ExitStatus(imp::WEXITSTATUS(status) as int) } else {