Merge #582

582: Add AioCb::from_boxed_slice r=posborne
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

Author: bors[bot]

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))
 

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<Box<[u8]>>),
+    /// 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<Box<[u8]>>,
+                          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()
     }
 }

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::sync::Mutex;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::{thread, time};
@@ -25,12 +27,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);
@@ -49,12 +51,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);
@@ -90,7 +92,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();
 
@@ -101,9 +103,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);
@@ -128,6 +130,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";
@@ -154,7 +181,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
@@ -165,28 +192,29 @@ 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";
 
     let mut f = tempfile().unwrap();
     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);
     aiocb.write().unwrap();
 
     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();
@@ -249,7 +277,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();
@@ -264,9 +292,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);
@@ -276,7 +304,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();
@@ -291,7 +319,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();
@@ -306,9 +334,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);
@@ -320,7 +348,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();
@@ -336,7 +364,7 @@ fn test_lio_listio_signal() {
     let _ = SIGUSR2_MTX.lock().expect("Mutex got poisoned by another test");
     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();
@@ -356,9 +384,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);
@@ -373,7 +401,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();
@@ -386,7 +414,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();