Double-free with into_owner

[steffahn]

use std::cell::Cell;

use self_cell::self_cell;

type O = Cell<Option<Box<u8>>>;

self_cell! {
    struct S {
        owner: O,
        #[covariant]
        dependent: D,
    }
}

struct D<'a>(&'a O);

impl Drop for D<'_> {
    fn drop(&mut self) {
        self.0.take();
    }
}

fn main() {
    let s = S::new(Cell::new(Some(Box::new(42))), |o| D(o));
    s.into_owner();
}

    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `target/debug/small_pg`
thread 'main' panicked at 'explicit panic', src/main.rs:20:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
free(): double free detected in tcache 2
[1]    394203 IOT instruction (core dumped)  cargo r

The underlying problem is

self_cell/src/unsafe_self_cell.rs

Lines 90 to 96 in ae22d0f

	let owner_ptr: *const Owner = &(*joined_ptr.as_ptr()).owner;
	// Move owner out so it can be returned.
	let owner = read(owner_ptr);
	// Clean up rest of JoinedCell
	drop_in_place(&mut (*joined_ptr.as_ptr()).dependent);

where the owner is read before dependent is dropped, so dropping dependent can access the original owner, but into_owner returns the copy created beforehand.

---

I have a branch that fixes this as well as some memory leaks: ebbf355...c482860

The branch is based on #21 (one of the memory leaks is in drop_joined), so I’ll only open a PR after #21 is merged. So consider not to release a new version yet after merging #21 😉

[steffahn]

Actually… I just noticed that a panic isn’t necessary in the example (and I edited the example accordingly). So I can split up the double-free issue and the memory leaks and create two separate branches; the first one (the double-free) can already get a PR right now.

[Voultapher]

Dang, thanks for finding this! Goes to show how hard it is to write unsafe Rust even with the best intentions and code review.