From f175ea7fd8f560daec7bc8c3c36b132be89ca23c Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 7 Apr 2017 23:30:47 -0600 Subject: [PATCH] Add AioCb::from_boxed_slice The existing AioCb constructors work for simple programs where everything is stored on the stack. But in more complicated programs the borrow checker can't prove that a buffer will outlive the AioCb that references it. Fix this problem by introducting AioCb::from_boxed_slice, which takes a reference-counted buffer. Fixes #575 --- CHANGELOG.md | 2 ++ src/sys/aio.rs | 57 ++++++++++++++++++++++++++----- test/sys/test_aio.rs | 80 ++++++++++++++++++++++++++++++-------------- 3 files changed, 105 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eac3072a9d..4826626493 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ### Added +- Added `AioCb::from_boxed_slice` + ([#582](https://github.com/nix-rust/nix/pull/582) - Added `nix::unistd::{openat, fstatat, readlink, readlinkat}` ([#551](https://github.com/nix-rust/nix/pull/551)) diff --git a/src/sys/aio.rs b/src/sys/aio.rs index 13a03b5dfd..b4d5a7e18c 100644 --- a/src/sys/aio.rs +++ b/src/sys/aio.rs @@ -8,6 +8,7 @@ use std::io::Write; use std::io::stderr; use std::marker::PhantomData; use std::mem; +use std::rc::Rc; use std::ptr::{null, null_mut}; use sys::signal::*; use sys::time::TimeSpec; @@ -61,6 +62,17 @@ pub enum AioCancelStat { AioAllDone = libc::AIO_ALLDONE, } +/// Private type used by nix to keep buffers from Drop'ing while the kernel has +/// a pointer to them. +#[derive(Clone, Debug)] +enum Keeper<'a> { + none, + /// Keeps a reference to a Boxed slice + boxed(Rc>), + /// Keeps a reference to a slice + phantom(PhantomData<&'a mut [u8]>) +} + /// The basic structure used by all aio functions. Each `aiocb` represents one /// I/O request. pub struct AioCb<'a> { @@ -69,7 +81,8 @@ pub struct AioCb<'a> { mutable: bool, /// Could this `AioCb` potentially have any in-kernel state? in_progress: bool, - phantom: PhantomData<&'a mut [u8]> + /// Used to keep buffers from Drop'ing + keeper: Keeper<'a> } impl<'a> AioCb<'a> { @@ -89,7 +102,7 @@ impl<'a> AioCb<'a> { a.aio_buf = null_mut(); let aiocb = AioCb { aiocb: a, mutable: false, in_progress: false, - phantom: PhantomData}; + keeper: Keeper::none}; aiocb } @@ -106,17 +119,43 @@ impl<'a> AioCb<'a> { /// which operation to use for this individual aiocb pub fn from_mut_slice(fd: RawFd, offs: off_t, buf: &'a mut [u8], prio: ::c_int, sigev_notify: SigevNotify, - opcode: LioOpcode) -> AioCb { + opcode: LioOpcode) -> AioCb<'a> { let mut a = AioCb::common_init(fd, prio, sigev_notify); a.aio_offset = offs; a.aio_nbytes = buf.len() as size_t; - // casting an immutable buffer to a mutable pointer looks unsafe, but - // technically its only unsafe to dereference it, not to create it. a.aio_buf = buf.as_ptr() as *mut c_void; a.aio_lio_opcode = opcode as ::c_int; let aiocb = AioCb { aiocb: a, mutable: true, in_progress: false, - phantom: PhantomData}; + keeper: Keeper::phantom(PhantomData)}; + aiocb + } + + /// Constructs a new `AioCb`. + /// + /// Unlike `from_mut_slice`, this method returns a structure suitable for + /// placement on the heap. + /// + /// * `fd` File descriptor. Required for all aio functions. + /// * `offs` File offset + /// * `buf` A shared memory buffer on the heap + /// * `prio` If POSIX Prioritized IO is supported, then the operation will + /// be prioritized at the process's priority level minus `prio` + /// * `sigev_notify` Determines how you will be notified of event + /// completion. + /// * `opcode` This field is only used for `lio_listio`. It determines + /// which operation to use for this individual aiocb + pub fn from_boxed_slice(fd: RawFd, offs: off_t, buf: Rc>, + prio: ::c_int, sigev_notify: SigevNotify, + opcode: LioOpcode) -> AioCb<'a> { + let mut a = AioCb::common_init(fd, prio, sigev_notify); + a.aio_offset = offs; + a.aio_nbytes = buf.len() as size_t; + a.aio_buf = buf.as_ptr() as *mut c_void; + a.aio_lio_opcode = opcode as ::c_int; + + let aiocb = AioCb{ aiocb: a, mutable: true, in_progress: false, + keeper: Keeper::boxed(buf)}; aiocb } @@ -139,12 +178,15 @@ impl<'a> AioCb<'a> { let mut a = AioCb::common_init(fd, prio, sigev_notify); a.aio_offset = offs; a.aio_nbytes = buf.len() as size_t; + // casting an immutable buffer to a mutable pointer looks unsafe, + // but technically its only unsafe to dereference it, not to create + // it. a.aio_buf = buf.as_ptr() as *mut c_void; assert!(opcode != LioOpcode::LIO_READ, "Can't read into an immutable buffer"); a.aio_lio_opcode = opcode as ::c_int; let aiocb = AioCb { aiocb: a, mutable: false, in_progress: false, - phantom: PhantomData}; + keeper: Keeper::none}; aiocb } @@ -284,7 +326,6 @@ impl<'a> Debug for AioCb<'a> { .field("aio_sigevent", &SigEvent::from(&self.aiocb.aio_sigevent)) .field("mutable", &self.mutable) .field("in_progress", &self.in_progress) - .field("phantom", &self.phantom) .finish() } } diff --git a/test/sys/test_aio.rs b/test/sys/test_aio.rs index 7f7a77193e..84768284ab 100644 --- a/test/sys/test_aio.rs +++ b/test/sys/test_aio.rs @@ -5,7 +5,9 @@ use nix::sys::aio::*; use nix::sys::signal::*; use nix::sys::time::{TimeSpec, TimeValLike}; use std::io::{Write, Read, Seek, SeekFrom}; +use std::ops::Deref; use std::os::unix::io::AsRawFd; +use std::rc::Rc; use std::{thread, time}; use tempfile::tempfile; @@ -23,12 +25,12 @@ fn poll_aio(mut aiocb: &mut AioCb) -> Result<()> { // AioCancelStat value. #[test] fn test_cancel() { - let mut wbuf = "CDEF".to_string().into_bytes(); + let wbuf: &'static [u8] = b"CDEF"; let f = tempfile().unwrap(); - let mut aiocb = AioCb::from_mut_slice( f.as_raw_fd(), + let mut aiocb = AioCb::from_slice( f.as_raw_fd(), 0, //offset - &mut wbuf, + &wbuf, 0, //priority SigevNotify::SigevNone, LioOpcode::LIO_NOP); @@ -47,12 +49,12 @@ fn test_cancel() { // Tests using aio_cancel_all for all outstanding IOs. #[test] fn test_aio_cancel_all() { - let mut wbuf = "CDEF".to_string().into_bytes(); + let wbuf: &'static [u8] = b"CDEF"; let f = tempfile().unwrap(); - let mut aiocb = AioCb::from_mut_slice( f.as_raw_fd(), + let mut aiocb = AioCb::from_slice(f.as_raw_fd(), 0, //offset - &mut wbuf, + &wbuf, 0, //priority SigevNotify::SigevNone, LioOpcode::LIO_NOP); @@ -88,7 +90,7 @@ fn test_aio_suspend() { const INITIAL: &'static [u8] = b"abcdef123456"; const WBUF: &'static [u8] = b"CDEF"; let timeout = TimeSpec::seconds(10); - let mut rbuf = vec![0; 4]; + let rbuf = Rc::new(vec![0; 4].into_boxed_slice()); let mut f = tempfile().unwrap(); f.write(INITIAL).unwrap(); @@ -99,9 +101,9 @@ fn test_aio_suspend() { SigevNotify::SigevNone, LioOpcode::LIO_WRITE); - let mut rcb = AioCb::from_mut_slice( f.as_raw_fd(), + let mut rcb = AioCb::from_boxed_slice( f.as_raw_fd(), 8, //offset - &mut rbuf, + rbuf.clone(), 0, //priority SigevNotify::SigevNone, LioOpcode::LIO_READ); @@ -126,6 +128,31 @@ fn test_aio_suspend() { // for completion #[test] fn test_read() { + const INITIAL: &'static [u8] = b"abcdef123456"; + let rbuf = Rc::new(vec![0; 4].into_boxed_slice()); + const EXPECT: &'static [u8] = b"cdef"; + let mut f = tempfile().unwrap(); + f.write(INITIAL).unwrap(); + { + let mut aiocb = AioCb::from_boxed_slice( f.as_raw_fd(), + 2, //offset + rbuf.clone(), + 0, //priority + SigevNotify::SigevNone, + LioOpcode::LIO_NOP); + aiocb.read().unwrap(); + + let err = poll_aio(&mut aiocb); + assert!(err == Ok(())); + assert!(aiocb.aio_return().unwrap() as usize == EXPECT.len()); + } + + assert!(EXPECT == rbuf.deref().deref()); +} + +// Tests from_mut_slice +#[test] +fn test_read_into_mut_slice() { const INITIAL: &'static [u8] = b"abcdef123456"; let mut rbuf = vec![0; 4]; const EXPECT: &'static [u8] = b"cdef"; @@ -152,7 +179,7 @@ fn test_read() { #[test] #[should_panic(expected = "Can't read into an immutable buffer")] fn test_read_immutable_buffer() { - let rbuf = vec![0; 4]; + let rbuf: &'static [u8] = b"CDEF"; let f = tempfile().unwrap(); let mut aiocb = AioCb::from_slice( f.as_raw_fd(), 2, //offset @@ -163,12 +190,13 @@ fn test_read_immutable_buffer() { aiocb.read().unwrap(); } + // Test a simple aio operation with no completion notification. We must poll // for completion. Unlike test_aio_read, this test uses AioCb::from_slice #[test] fn test_write() { const INITIAL: &'static [u8] = b"abcdef123456"; - const WBUF: &'static [u8] = b"CDEF"; //"CDEF".to_string().into_bytes(); + let wbuf = "CDEF".to_string().into_bytes(); let mut rbuf = Vec::new(); const EXPECT: &'static [u8] = b"abCDEF123456"; @@ -176,7 +204,7 @@ fn test_write() { f.write(INITIAL).unwrap(); let mut aiocb = AioCb::from_slice( f.as_raw_fd(), 2, //offset - &WBUF, + &wbuf, 0, //priority SigevNotify::SigevNone, LioOpcode::LIO_NOP); @@ -184,7 +212,7 @@ fn test_write() { let err = poll_aio(&mut aiocb); assert!(err == Ok(())); - assert!(aiocb.aio_return().unwrap() as usize == WBUF.len()); + assert!(aiocb.aio_return().unwrap() as usize == wbuf.len()); f.seek(SeekFrom::Start(0)).unwrap(); let len = f.read_to_end(&mut rbuf).unwrap(); @@ -245,7 +273,7 @@ fn test_write_sigev_signal() { fn test_lio_listio_wait() { const INITIAL: &'static [u8] = b"abcdef123456"; const WBUF: &'static [u8] = b"CDEF"; - let mut rbuf = vec![0; 4]; + let rbuf = Rc::new(vec![0; 4].into_boxed_slice()); let mut rbuf2 = Vec::new(); const EXPECT: &'static [u8] = b"abCDEF123456"; let mut f = tempfile().unwrap(); @@ -260,9 +288,9 @@ fn test_lio_listio_wait() { SigevNotify::SigevNone, LioOpcode::LIO_WRITE); - let mut rcb = AioCb::from_mut_slice( f.as_raw_fd(), + let mut rcb = AioCb::from_boxed_slice( f.as_raw_fd(), 8, //offset - &mut rbuf, + rbuf.clone(), 0, //priority SigevNotify::SigevNone, LioOpcode::LIO_READ); @@ -272,7 +300,7 @@ fn test_lio_listio_wait() { assert!(wcb.aio_return().unwrap() as usize == WBUF.len()); assert!(rcb.aio_return().unwrap() as usize == WBUF.len()); } - assert!(rbuf == b"3456"); + assert!(rbuf.deref().deref() == b"3456"); f.seek(SeekFrom::Start(0)).unwrap(); let len = f.read_to_end(&mut rbuf2).unwrap(); @@ -287,7 +315,7 @@ fn test_lio_listio_wait() { fn test_lio_listio_nowait() { const INITIAL: &'static [u8] = b"abcdef123456"; const WBUF: &'static [u8] = b"CDEF"; - let mut rbuf = vec![0; 4]; + let rbuf = Rc::new(vec![0; 4].into_boxed_slice()); let mut rbuf2 = Vec::new(); const EXPECT: &'static [u8] = b"abCDEF123456"; let mut f = tempfile().unwrap(); @@ -302,9 +330,9 @@ fn test_lio_listio_nowait() { SigevNotify::SigevNone, LioOpcode::LIO_WRITE); - let mut rcb = AioCb::from_mut_slice( f.as_raw_fd(), + let mut rcb = AioCb::from_boxed_slice( f.as_raw_fd(), 8, //offset - &mut rbuf, + rbuf.clone(), 0, //priority SigevNotify::SigevNone, LioOpcode::LIO_READ); @@ -316,7 +344,7 @@ fn test_lio_listio_nowait() { assert!(wcb.aio_return().unwrap() as usize == WBUF.len()); assert!(rcb.aio_return().unwrap() as usize == WBUF.len()); } - assert!(rbuf == b"3456"); + assert!(rbuf.deref().deref() == b"3456"); f.seek(SeekFrom::Start(0)).unwrap(); let len = f.read_to_end(&mut rbuf2).unwrap(); @@ -331,7 +359,7 @@ fn test_lio_listio_nowait() { fn test_lio_listio_signal() { const INITIAL: &'static [u8] = b"abcdef123456"; const WBUF: &'static [u8] = b"CDEF"; - let mut rbuf = vec![0; 4]; + let rbuf = Rc::new(vec![0; 4].into_boxed_slice()); let mut rbuf2 = Vec::new(); const EXPECT: &'static [u8] = b"abCDEF123456"; let mut f = tempfile().unwrap(); @@ -351,9 +379,9 @@ fn test_lio_listio_signal() { SigevNotify::SigevNone, LioOpcode::LIO_WRITE); - let mut rcb = AioCb::from_mut_slice( f.as_raw_fd(), + let mut rcb = AioCb::from_boxed_slice( f.as_raw_fd(), 8, //offset - &mut rbuf, + rbuf.clone(), 0, //priority SigevNotify::SigevNone, LioOpcode::LIO_READ); @@ -368,7 +396,7 @@ fn test_lio_listio_signal() { assert!(wcb.aio_return().unwrap() as usize == WBUF.len()); assert!(rcb.aio_return().unwrap() as usize == WBUF.len()); } - assert!(rbuf == b"3456"); + assert!(rbuf.deref().deref() == b"3456"); f.seek(SeekFrom::Start(0)).unwrap(); let len = f.read_to_end(&mut rbuf2).unwrap(); @@ -381,7 +409,7 @@ fn test_lio_listio_signal() { #[cfg(not(any(target_os = "ios", target_os = "macos")))] #[should_panic(expected = "Can't read into an immutable buffer")] fn test_lio_listio_read_immutable() { - let rbuf = vec![0; 4]; + let rbuf: &'static [u8] = b"abcd"; let f = tempfile().unwrap();