0

This is something of a follow-up to a previous question of mine. TL;DR: I tried to create a self-referencing struct by heap allocating the self-referencee using a Box. It was pointed out that I can't rely on the pointer provenance not changing (it is currently undecided).

This led to me trying to implement a sort of custom Box, PinnedClient, which allocates to the heap when created and deallocates on Drop:

struct PinnedClient(pub *mut Client);

impl PinnedClient {
    unsafe fn new(client: Client) -> PinnedClient {
        // Allocate memory on the heap
        let layout = Layout::new::<Client>();
        let pointer = alloc(layout) as *mut Client;

        // Make sure it worked
        if pointer.is_null() {
            handle_alloc_error(layout);
        }

        // Move the client object to the heap
        pointer.write(client);

        // Return a `PinnedClient` object with a pointer
        // to the underlying client.
        PinnedClient(pointer)
    }
}

impl Drop for PinnedClient {
    fn drop(&mut self) {
        // Deallocate the previously allocated when
        // wrapper is dropped.
        unsafe { 
            dealloc(self.0 as *mut u8, Layout::new::<Client>());
        }
    }
}

Then I include a PinnedClient in my struct and use the raw pointer to create the self-reference:

pub struct MyTransaction<'a> {
    transaction: Transaction<'a>,
    _client: PinnedClient
}

impl<'a> MyTransaction<'a> {
    async fn from_client<'this>(client: Client) -> Result<MyTransaction<'a>, Error> {
        let client = unsafe { PinnedClient::new(client) };
        let transaction = unsafe {
            // Convert `*mut Client` to `&mut Client`
            // This shouldn't fail since the pointer in PinnedCliend
            // is guaranteed not to be null.
            &mut *client.0
        }.transaction().await?;

        Ok(
            MyTransaction { _client: client, transaction }
        )
    }
}

Now I am wondering:

  • is it undefined behaviour to "hand out" a mutable reference to the Client and
  • is it it actually guaranteed that the memory gets deallocated (does dealloc get called in all scenarios)?

I am somewhat anxious since self-referential structs are supposed to be hard and I feel like I am missing something.

Einliterflasche
  • 473
  • 6
  • 18
  • 1
    Note that I didn't say that every time the box is moved its uniqueness is asserted, I said that this _might_ be the case (not decided yet). From a practical point of view, you should approach it as if it was decided positively, to be safe against such future decision. – Chayim Friedman Aug 11 '23 at 12:56
  • This first statement is again wrong: the _address_ of the object won't change. That's a basic fact driven from the fact that `Box` is heap-allocated and it is not allowed to move the memory behind your back. What can change is the _provenance_, what giving you access rights to this place with your reference/pointer. – Chayim Friedman Aug 11 '23 at 13:16

1 Answers1

0

is it undefined behaviour to "hand out" a mutable reference to the Client

No it is not (of course, assuming you keep all aliasing rules and do not hand out two such references).

However, as pointed out in your previous questions, you need to swap the fields, as currently the Transaction gets dropped after the Client but it may access it during its drop, causing a use-after-free.

is it it actually guaranteed that the memory gets deallocated (does dealloc get called in all scenarios)?

Yes and no.

No, because in Rust, destructors are not guaranteed to be called (barring special requirements of Pin). If the destructor of MyTransaction does not run (because of std::mem::forget() or because of a Rc cycle or because of whatever), then the memory will not be deallocated.

But also yes, because the answer for the question you've probably meant to ask is yes: every time the memory would be deallocated if the pointer was Box, is will also be deallocated with your smart pointer. Box also uses Drop, it is not special (it is special in other ways, though).

However...

You missed it again. Your code still leaks resources.

The backing memory for the Client will be deallocated, but the Client itself won't be dropped. This means that if your client owns some resources (heap memory, file descriptors, network connections, etc.) they won't be released.

Even if currently Client doesn't have any drop glue, if it is coming from a library you don't control, nothing promises it will continue to not have drop glue in the future. Adding a Drop impl is not considered a breaking change. So you still need to drop it, to be future-proof.

Fortunately, this is easy:

impl Drop for PinnedClient {
    fn drop(&mut self) {
        // Deallocate the previously allocated when
        // wrapper is dropped.
        unsafe { 
            self.0.drop_in_place();
            dealloc(self.0 as *mut u8, Layout::new::<Client>());
        }
    }
}

since self-referential structs are supposed to be hard

You asked two questions here, you failed subtley twice to write UB-free code. Is this not hard enough for you?

Chayim Friedman
  • 47,971
  • 5
  • 48
  • 77
  • Thank you. I just want to remark that I fixed the field order for this question, I actually used `drop_in_place` but excluded it for the example and I didn't say self-referential structs are easy. – Einliterflasche Aug 11 '23 at 13:44