diff options
| -rw-r--r-- | CHANGELOG.md | 4 | ||||
| -rw-r--r-- | Cargo.toml | 4 | ||||
| -rw-r--r-- | src/sys/aio.rs | 43 | ||||
| -rw-r--r-- | test/sys/test_aio.rs | 85 | ||||
| -rw-r--r-- | test/sys/test_aio_drop.rs | 27 |
5 files changed, 109 insertions, 54 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 36b528ca..d5d4528b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,10 +51,14 @@ This project adheres to [Semantic Versioning](http://semver.org/). ([#744](https://github.com/nix-rust/nix/pull/744)) - Moved constants ptrace request, event and options to enums and updated ptrace functions and argument types accordingly. ([#749](https://github.com/nix-rust/nix/pull/749)) +- `AioCb::Drop` will now panic if the `AioCb` is still in-progress ([#715](https://github.com/nix-rust/nix/pull/715)) # Fixed - Fix compilation and tests for OpenBSD targets ([#688](https://github.com/nix-rust/nix/pull/688)) +- Fixed error handling in `AioCb::fsync`, `AioCb::read`, and `AioCb::write`. + It is no longer an error to drop an `AioCb` that failed to enqueue in the OS. + ([#715](https://github.com/nix-rust/nix/pull/715)) # Removed - The syscall module has been removed. This only exposed enough functionality for @@ -32,6 +32,10 @@ name = "test" path = "test/test.rs" [[test]] +name = "test-aio-drop" +path = "test/sys/test_aio_drop.rs" + +[[test]] name = "test-mount" path = "test/test_mount.rs" harness = false diff --git a/src/sys/aio.rs b/src/sys/aio.rs index abb742f3..22bd3959 100644 --- a/src/sys/aio.rs +++ b/src/sys/aio.rs @@ -4,8 +4,6 @@ use libc::{c_void, off_t, size_t}; use libc; use std::fmt; use std::fmt::Debug; -use std::io::Write; -use std::io::stderr; use std::marker::PhantomData; use std::mem; use std::rc::Rc; @@ -234,16 +232,22 @@ impl<'a> AioCb<'a> { /// An asynchronous version of `fsync`. pub fn fsync(&mut self, mode: AioFsyncMode) -> Result<()> { let p: *mut libc::aiocb = &mut self.aiocb; - self.in_progress = true; - Errno::result(unsafe { libc::aio_fsync(mode as libc::c_int, p) }).map(drop) + Errno::result(unsafe { + libc::aio_fsync(mode as libc::c_int, p) + }).map(|_| { + self.in_progress = true; + }) } /// Asynchronously reads from a file descriptor into a buffer pub fn read(&mut self) -> Result<()> { assert!(self.mutable, "Can't read into an immutable buffer"); let p: *mut libc::aiocb = &mut self.aiocb; - self.in_progress = true; - Errno::result(unsafe { libc::aio_read(p) }).map(drop) + Errno::result(unsafe { + libc::aio_read(p) + }).map(|_| { + self.in_progress = true; + }) } /// Retrieve return status of an asynchronous operation. Should only be @@ -259,8 +263,11 @@ impl<'a> AioCb<'a> { /// Asynchronously writes from a buffer to a file descriptor pub fn write(&mut self) -> Result<()> { let p: *mut libc::aiocb = &mut self.aiocb; - self.in_progress = true; - Errno::result(unsafe { libc::aio_write(p) }).map(drop) + Errno::result(unsafe { + libc::aio_write(p) + }).map(|_| { + self.in_progress = true; + }) } } @@ -332,24 +339,8 @@ impl<'a> Debug for AioCb<'a> { impl<'a> Drop for AioCb<'a> { /// If the `AioCb` has no remaining state in the kernel, just drop it. - /// Otherwise, collect its error and return values, so as not to leak - /// resources. + /// Otherwise, dropping constitutes a resource leak, which is an error fn drop(&mut self) { - if self.in_progress { - // Well-written programs should never get here. They should always - // wait for an AioCb to complete before dropping it - let _ = write!(stderr(), "WARNING: dropped an in-progress AioCb"); - loop { - let ret = aio_suspend(&[&self], None); - match ret { - Ok(()) => break, - Err(Error::Sys(Errno::EINVAL)) => panic!( - "Inconsistent AioCb.in_progress value"), - Err(Error::Sys(Errno::EINTR)) => (), // Retry interrupted syscall - _ => panic!("Unexpected aio_suspend return value {:?}", ret) - }; - } - let _ = self.aio_return(); - } + assert!(!self.in_progress, "Dropped an in-progress AioCb"); } } diff --git a/test/sys/test_aio.rs b/test/sys/test_aio.rs index e4b48b8d..67fd0850 100644 --- a/test/sys/test_aio.rs +++ b/test/sys/test_aio.rs @@ -88,6 +88,26 @@ fn test_fsync() { aiocb.aio_return().unwrap(); } +/// `AioCb::fsync` should not modify the `AioCb` object if libc::aio_fsync returns +/// an error +// Skip on Linux, because Linux's AIO implementation can't detect errors +// synchronously +#[test] +#[cfg(any(target_os = "freebsd", target_os = "macos"))] +fn test_fsync_error() { + use std::mem; + + const INITIAL: &'static [u8] = b"abcdef123456"; + // Create an invalid AioFsyncMode + let mode = unsafe { mem::transmute(666) }; + let mut f = tempfile().unwrap(); + f.write(INITIAL).unwrap(); + let mut aiocb = AioCb::from_fd( f.as_raw_fd(), + 0, //priority + SigevNotify::SigevNone); + let err = aiocb.fsync(mode); + assert!(err.is_err()); +} #[test] #[cfg_attr(all(target_env = "musl", target_arch = "x86_64"), ignore)] @@ -156,6 +176,26 @@ fn test_read() { assert!(EXPECT == rbuf.deref().deref()); } +/// `AioCb::read` should not modify the `AioCb` object if libc::aio_read returns +/// an error +// Skip on Linux, because Linux's AIO implementation can't detect errors +// synchronously +#[test] +#[cfg(any(target_os = "freebsd", target_os = "macos"))] +fn test_read_error() { + const INITIAL: &'static [u8] = b"abcdef123456"; + let rbuf = Rc::new(vec![0; 4].into_boxed_slice()); + let mut f = tempfile().unwrap(); + f.write(INITIAL).unwrap(); + let mut aiocb = AioCb::from_boxed_slice( f.as_raw_fd(), + -1, //an invalid offset + rbuf.clone(), + 0, //priority + SigevNotify::SigevNone, + LioOpcode::LIO_NOP); + assert!(aiocb.read().is_err()); +} + // Tests from_mut_slice #[test] #[cfg_attr(all(target_env = "musl", target_arch = "x86_64"), ignore)] @@ -230,6 +270,23 @@ fn test_write() { assert!(rbuf == EXPECT); } +/// `AioCb::write` should not modify the `AioCb` object if libc::aio_write returns +/// an error +// Skip on Linux, because Linux's AIO implementation can't detect errors +// synchronously +#[test] +#[cfg(any(target_os = "freebsd", target_os = "macos"))] +fn test_write_error() { + let wbuf = "CDEF".to_string().into_bytes(); + let mut aiocb = AioCb::from_slice( 666, // An invalid file descriptor + 0, //offset + &wbuf, + 0, //priority + SigevNotify::SigevNone, + LioOpcode::LIO_NOP); + assert!(aiocb.write().is_err()); +} + lazy_static! { pub static ref SIGNALED: AtomicBool = AtomicBool::new(false); } @@ -442,31 +499,3 @@ fn test_lio_listio_read_immutable() { LioOpcode::LIO_READ); let _ = lio_listio(LioMode::LIO_NOWAIT, &[&mut rcb], SigevNotify::SigevNone); } - -// Test dropping an AioCb that hasn't yet finished. Behind the scenes, the -// library should wait for the AioCb's completion. -#[test] -#[cfg_attr(all(target_env = "musl", target_arch = "x86_64"), ignore)] -fn test_drop() { - const INITIAL: &'static [u8] = b"abcdef123456"; - const WBUF: &'static [u8] = b"CDEF"; //"CDEF".to_string().into_bytes(); - let mut rbuf = Vec::new(); - const EXPECT: &'static [u8] = b"abCDEF123456"; - - let mut f = tempfile().unwrap(); - f.write(INITIAL).unwrap(); - { - let mut aiocb = AioCb::from_slice( f.as_raw_fd(), - 2, //offset - &WBUF, - 0, //priority - SigevNotify::SigevNone, - LioOpcode::LIO_NOP); - aiocb.write().unwrap(); - } - - f.seek(SeekFrom::Start(0)).unwrap(); - let len = f.read_to_end(&mut rbuf).unwrap(); - assert!(len == EXPECT.len()); - assert!(rbuf == EXPECT); -} diff --git a/test/sys/test_aio_drop.rs b/test/sys/test_aio_drop.rs new file mode 100644 index 00000000..ef0f5041 --- /dev/null +++ b/test/sys/test_aio_drop.rs @@ -0,0 +1,27 @@ +extern crate nix; +extern crate tempfile; + +use nix::sys::aio::*; +use nix::sys::signal::*; +use std::os::unix::io::AsRawFd; +use tempfile::tempfile; + +// Test dropping an AioCb that hasn't yet finished. +// This must happen in its own process, because on OSX this test seems to hose +// the AIO subsystem and causes subsequent tests to fail +#[test] +#[should_panic(expected = "Dropped an in-progress AioCb")] +#[cfg(not(target_env = "musl"))] +fn test_drop() { + const WBUF: &'static [u8] = b"CDEF"; + + let f = tempfile().unwrap(); + f.set_len(6).unwrap(); + let mut aiocb = AioCb::from_slice( f.as_raw_fd(), + 2, //offset + &WBUF, + 0, //priority + SigevNotify::SigevNone, + LioOpcode::LIO_NOP); + aiocb.write().unwrap(); +} |