Implement OwningHandle

[bholley]

This is the extension to owning_ref that I wrote for Servo to overcome the issue we discussed a few weeks ago.

r? @Kimundi

[bholley]

CC @SimonSapin

[bholley]

Per discussion on IRC with @SimonSapin, we decided that we could insulate most callers from the unsafety by specializing OwningHandle for various common types. So we could have new_ref, new_refmut, new_rwlockreadguard, etc, which don't take a callback and thus keep the unsafety scoped within OwningHandle.

I'd like to leave the generic option in place though, because it makes it easy to use OwningHandle on custom types.

[Kimundi]

Looks good, and I also can't seem to break it, so in it goes!

[jpernst]

If this is in a workable state, would it be possible to get a new release published with this available? It solves exactly the problem I've run into, and I'm sure others will find it useful as well.

[bholley]

We should probably also merge #17 and #16.

[jpernst]

An issue I've run into while using this is that it's possible for handle creation to fail, but there's no way to communicate that from the closure. One workaround is to just create the handle ahead of time and mem::transmute it in the closure, but that means the closure argument is unrelated to the handle, which seems fragile. Instead I elected to first create a bounded handle to make sure it succeeds, then throw it away, then create a new one with unwrap inside the closure. This isn't ideal in my case since the handle creation is expensive. Perhaps there should be a method such as OwningHandle::try_new that returns a Result with the error type inferred from the result type of the closure.

[bholley]

try_new seems reasonable to me. Worth reconciling with #17.

[jpernst]

While exploring this further, I'm starting to suspect that this API is fundamentally unsafe.

Example:

#![feature(cell_extras)]

extern crate owning_ref;

use std::cell::{RefCell, Ref};

#[derive(Debug)]
struct Foo;

fn main() {
    let freed = {
        let br = Box::new(RefCell::new(Foo));
        let oh = owning_ref::OwningHandle::new(br, |br| Box::new(unsafe { (*br).borrow() }));
        Ref::clone(&*oh)
    };

    println!("{:?}", freed);
}

The requirement for H: Deref only prevents one layer of escaping. By just adding a Box or Arc we can defeat that protection and expose whatever is inside. I'm not sure if this can be prevented in any reasonable way. One could argue that the closure is unsafe anyway so all bets are off, but that's to prevent escaping. Even if we could somehow prevent escaping from the closure, the "fake" lifetime can still be exposed and misused if it appears as a nested argument of a generic type.

It may be necessary to just declare this API to be inherently unsafe with a warning about what guarantees you need to make for it to actually be safe to consume.