Am I ok to implement 'Send' for this struct?

Hi, I've written a wrapper around some mmap-d memory which I would like to be able to move to another thread (without requiring concurrent access), however I'm currently bumping heads with the compiler as it requires me to implement Send on my wrapper before I can do this.

I've included my implementation below and was wondering if someone would be able to take a quick look over it and check if it indeed makes sense to implement Send?

I've thought about this a bit and feel like I should be fine as the wrapper owns the raw pointer and there is no way to access it directly, and the provided memory access functions ensure that there is only one mutable borrow in existence at once, however I'm yet to get a feel for this kind of stuff in Rust so just wanted to double check I'm not doing anything mad.

Thanks in advance!

use std::{convert::TryInto, io, ptr, slice};
use log::error;

#[derive(Debug)]
pub enum MmapAccessError {
    OffsetOutOfBounds,
    MemRangeOutOfBounds,
}

pub struct MmapArea {
    len: usize,
    mem_ptr: *mut libc::c_void,
}

// Scary bit
unsafe impl Send for MmapArea {}

impl MmapArea {
    pub fn new(len: usize, use_huge_pages: bool) -> io::Result<Self> {
        // Create mmap and raw pointer
    }

     pub fn mem_range(&self, offset: usize, len: usize) -> Result<&[u8], MmapAccessError> {
        self.check_bounds(offset, len)?;

        unsafe {
            let ptr = self.mem_ptr.offset(offset.try_into().unwrap());

            Ok(slice::from_raw_parts(ptr as *const u8, len))
        }
    }

    pub fn mem_range_mut(
        &mut self,
        offset: usize,
        len: usize,
    ) -> Result<&mut [u8], MmapAccessError> {
        self.check_bounds(offset, len)?;

        unsafe {
            let ptr = self.mem_ptr.offset(offset.try_into().unwrap());

            Ok(slice::from_raw_parts_mut(ptr as *mut u8, len))
        }
    }

    fn check_bounds(&self, offset: usize, len: usize) -> Result<(), MmapAccessError> {
        if offset >= self.len {
            return Err(MmapAccessError::OffsetOutOfBounds);
        } else if offset + len > self.len {
            return Err(MmapAccessError::MemRangeOutOfBounds);
        }
        Ok(())
    }
}

impl Drop for MmapArea {
    fn drop(&mut self) {
        let err = unsafe { libc::munmap(self.mem_ptr, self.len) };

        if err != 0 {
            error!("munmap failed: {}", err);
        }
    }
}

Yes, it's fine. Implementing Send (not Sync) doesn't have many gotchas. As long as nothing depends on thread's identity (thread-local storage, etc.) or shared mutable state (like Rc), it's safe to implement Send.

1 Like

Awesome, I appreciate your taking the time to have a look. And thanks for the pointers, it's made things a bit more concrete as to when I should be concerned with adding Send.

Those guidelines are fine for raw data, but you should still make sure not to violate the safety of other types you contain. For example, if you have some &T, then your Send must depend on T: Sync.

2 Likes

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.