diff options
author | bors[bot] <bors[bot]@users.noreply.github.com> | 2017-07-16 05:08:06 +0000 |
---|---|---|
committer | bors[bot] <bors[bot]@users.noreply.github.com> | 2017-07-16 05:08:06 +0000 |
commit | 5d296505b597d29bf6b339904f7f1550febe863d (patch) | |
tree | 4c493ed698e5f68a58603643cf7db1daed284d2f /src | |
parent | f84a008559d4f8ce0a79652ad0c515e7139a272c (diff) | |
parent | 5fb4cebcc6c9b241668d36c8732c93e1978b135e (diff) | |
download | nix-5d296505b597d29bf6b339904f7f1550febe863d.zip |
Merge #677
677: PtyMaster::drop should panic on EBADF r=asomers
Fixes #659
Diffstat (limited to 'src')
-rw-r--r-- | src/pty.rs | 15 | ||||
-rw-r--r-- | src/unistd.rs | 34 |
2 files changed, 45 insertions, 4 deletions
@@ -45,10 +45,17 @@ impl IntoRawFd for PtyMaster { impl Drop for PtyMaster { fn drop(&mut self) { - // Errors when closing are ignored because we don't actually know if the file descriptor - // was closed. If we retried, it's possible that descriptor was reallocated in the mean - // time and the wrong file descriptor could be closed. - let _ = ::unistd::close(self.0); + // On drop, we ignore errors like EINTR and EIO because there's no clear + // way to handle them, we can't return anything, and (on FreeBSD at + // least) the file descriptor is deallocated in these cases. However, + // we must panic on EBADF, because it is always an error to close an + // invalid file descriptor. That frequently indicates a double-close + // condition, which can cause confusing errors for future I/O + // operations. + let e = ::unistd::close(self.0); + if e == Err(Error::Sys(Errno::EBADF)) { + panic!("Closing an invalid file descriptor!"); + }; } } diff --git a/src/unistd.rs b/src/unistd.rs index 3209bb98..196232b6 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -670,6 +670,40 @@ pub fn gethostname<'a>(buffer: &'a mut [u8]) -> Result<&'a CStr> { }) } +/// Close a raw file descriptor +/// +/// Be aware that many Rust types implicitly close-on-drop, including +/// `std::fs::File`. Explicitly closing them with this method too can result in +/// a double-close condition, which can cause confusing `EBADF` errors in +/// seemingly unrelated code. Caveat programmer. +/// +/// # Examples +/// +/// ```no_run +/// extern crate tempfile; +/// extern crate nix; +/// +/// use std::os::unix::io::AsRawFd; +/// use nix::unistd::close; +/// +/// fn main() { +/// let mut f = tempfile::tempfile().unwrap(); +/// close(f.as_raw_fd()).unwrap(); // Bad! f will also close on drop! +/// } +/// ``` +/// +/// ```rust +/// extern crate tempfile; +/// extern crate nix; +/// +/// use std::os::unix::io::IntoRawFd; +/// use nix::unistd::close; +/// +/// fn main() { +/// let mut f = tempfile::tempfile().unwrap(); +/// close(f.into_raw_fd()).unwrap(); // Good. into_raw_fd consumes f +/// } +/// ``` pub fn close(fd: RawFd) -> Result<()> { let res = unsafe { libc::close(fd) }; Errno::result(res).map(drop) |