diff options
author | bors[bot] <bors[bot]@users.noreply.github.com> | 2017-09-03 23:53:27 +0000 |
---|---|---|
committer | bors[bot] <bors[bot]@users.noreply.github.com> | 2017-09-03 23:53:27 +0000 |
commit | 413fcabdef0af55d5bc1a649bc3af221600ce171 (patch) | |
tree | bbc44568c20a417cc19c37e0de4eddf6894c691c /src | |
parent | 219fcd7adaa7cecd178347782103c9b9cec86c58 (diff) | |
parent | bd2cda18cec3d2acb1f7bb37778be557029b0268 (diff) | |
download | nix-413fcabdef0af55d5bc1a649bc3af221600ce171.zip |
Merge #715
715: Fix multiple issues with POSIX AIO r=asomers a=asomers
Fixed error handling in `AioCb::fsync`, `AioCb::read`, and `AioCb::write`
Previously, the `AioCb`'s `in_progress` field would erroneously be set
to `true`, even if the syscall had an error
Fixes #714
AioCb::Drop will now panic for in-progress AioCb
Printing a warning message to stderr isn't really appropriate, because
there's no way to guarantee that stderr is even valid. Nor is
aio_suspend necessarily an appropriate action to take.
Diffstat (limited to 'src')
-rw-r--r-- | src/sys/aio.rs | 43 |
1 files changed, 17 insertions, 26 deletions
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"); } } |