Error handling with cleanup reversely

code review
1

Hello Rust forum:

I got a design question about error handling. I am developing my personal project rrxv6, and I encounter this in kvm design.
When a user process is being allocated, it has to map two pages in page table.
However if the mapping failed, it has to free that page in page table.
The code is here

The error handling part is un-implemented and TODO comment are there right now.
I do know there is railway oriented programming. However it does not quite fit my scenarios since I will need to handle the error reversely.
Hope someone can give me some advice on how to write this clean and neat.

2

What does it mean to handle error "reversely"?
Your code seems to be fairly clean and neat at the moment.

3

That is:

  1. map page a
    -> success, go to 2
    -> if error, free page a, return Err
  2. map page a
    -> success, go to 3
    -> if error, free page b AND page a, return Err
  3. return OK
    So if I am going to map more pages, the error handling will become longer and longer, which is not good.
4

Unfortunately, when you're working with functions that have an effective C-like semantic, like kalloc and uvfree and uvunmap, you have very little choice other than repeating code.
A solution would be to make a Box-like wrapper around kalloc and uvfree - that way you can ensure that memory is always freed.
I am guessing you need a similar idea for uvunmap although I don't have enough domain knowledge to suggest a wrapper for that.

1 Like
5

The best way to handle this is with the RAII pattern.

The idea is you create some object which represents a mapped page, and you unmap the page as part of its Drop implementation. Local variables are dropped in the opposite order to how they were created, so this should automatically give you the stack-like semantics you want.

As a bonus, when you use a RAII guard you could allocate 50 different pages and wouldn't need to add any extra cleanup code because the compiler makes sure all variables are dropped correctly.

One implementation might look like this:

struct Page {
  start: NonNull<u8>,
  len: usize,
}

impl Page {
  fn allocate(size: usize) -> Result<Self, AllocationError> {
    let ptr = unsafe { malloc(size) };
    match NonNull::new(ptr) {
      Some(start) => Ok(Page { start, len: size }),
      None => Err(AllocationError),
    }
  }
}

impl Drop for Page {
  fn drop(&mut self) {
    unsafe { free(self.ptr.as_ptr()); }
  }
}

struct AllocationError;

We can also add a bunch of helpers for working with pages:

impl Page {
  fn start(&self) -> NonNull<u8> { self.start }

  fn bytes(&self) -> &[MaybeUninit<u8>] {
    unsafe { std::slice::from_raw_parts(self.start.as_ptr().cast(), self.len) }
  }

  fn bytes_mut(&self) -> &mut [MaybeUninit<u8>] {
    unsafe { std::slice::from_raw_parts_mut(self.start.as_ptr().cast(), self.len) }
  }

  /// Consume the `Page` object *without* calling its destructor and unmapping it. 
  fn leak(self) {
    std::mem::forget(self);
  }
}
2 Likes
6

I think you are right, I will refactor my code and see if it fits my implementation.

1 Like
7

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.