How does this function increase refcount?

1 
unsafe fn increase_refcount<T: ArcWake>(data: *const ()) {
    // Retain Arc, but don't touch refcount by wrapping in ManuallyDrop
    let arc = mem::ManuallyDrop::new(Arc::<T>::from_raw(data.cast::<T>()));
    // Now increase refcount, but don't drop new refcount either
    let _arc_clone: mem::ManuallyDrop<_> = arc.clone();
}

I am a bit baffled by what is going on here. The current best approximation is:

  1. normally, when arc and _arc_clone gets dropped, the ref count goes -1

  2. the first ManuallyDrop prevents arc drop handler from being called

  3. the scond ManuallyDrop prevents _arc_clone drop handler from being called

  4. the arc.clone() line is where the count is actually incremented

Is this what is going on? If so, does this look a bit convoluted? I feel like there might be a more straight forward way to express this.

2

Arc::increment_strong_count can also be used here, which does the same thing. It might be the case that this method was not available at the time.

2 Likes
3

That's RAII for you.

Going back in time before ManuallyDrop

// cast added 1.38, from_raw needs 1.17
let arc = Arc::from_raw(data as *const T));
let arc_clone = arc.clone();
std::mem::forget(arc_clone); // could just make into one liner without added variable
std::mem::forget(arc); // still have the arc from prior into_raw stored in data
4

I actually find this impl clearer. I think the thing that threw me off was the name ManuallyDrop. Perhaps if it was named IgnoreDrop or DontAutoDrop, it would be clearer.

I get that ManuallyDrop signifies: it's your job to manually call drop on it if you want to call drop; but still adjusting to the naming.

5

The problem with explicit forget is that it's not unwind-safe. This might not actually be a problem in this particular piece of code, but in general, in the presence of unsafe, calling too many destructors because forget() wasn't invoked due to a panic is a real problem.

1 Like
6

Can you provide a minimal example of when forget is a problem but ManuallyDrop "works" ?

7

Sure!

let resource = get_resource();
let managed_ptr = resource.get();
unsafe {
    // if this panics, `resource.drop()` is called: potential double free
    free_managed_ptr(managed_ptr);
}
mem::forget(resource);
1 Like
8

In the same file, there’s a good example

unsafe fn wake_by_ref_arc_raw<T: ArcWake>(data: *const ()) {
    // Retain Arc, but don't touch refcount by wrapping in ManuallyDrop
    let arc = mem::ManuallyDrop::new(Arc::<T>::from_raw(data.cast::<T>()));
    ArcWake::wake_by_ref(&arc);
}

It that was instead

unsafe fn wake_by_ref_arc_raw<T: ArcWake>(data: *const ()) {
    let arc = Arc::<T>::from_raw(data.cast::<T>());
    ArcWake::wake_by_ref(&arc);
    mem::forget(arc);
}

then the created Arc would be dropped when the ArcWake::wake_by_ref call panics, which could easily cause use-after-free errors or similar down the line.

1 Like
9

@H2CO3 @steffahn : Thanks for the examples.

So the highlight is:

a1: let x = ... ;
a2: let x = ManuallyDrop::new(x);
a3-a:10: do stuff with x

b1: let x = ...;
b2: // 
b3-b10: do stuff with x
b11: std::mem::forget(x)

To exec the drop handler in ManuallyDrop, we need something bad to happen between a1-a2; while an unwind happening in a3-a10 won't effect it.

To exec the drop handler in std::mem::forget; if an unwind happens in b3-b10, that suffices.

This is the main distinction ?

10

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.