Possible to leak a non-static reference into a sendable proc

[sfackler]

This successfully compiles, even though inner is a &Foo.

use std::sync::TaskPool;

struct Foo {
    p: TaskPool,
    q: uint
}

impl Foo {
    fn f(&self) {
        let inner = self.clone();
        self.p.execute(proc() {
            println!("{}", inner.q);
        });
    }
}

fn main() {}

For a more real example, see sfackler/r2d2@d864a78. It'll cause tests to segfault ~50% of the time.

[aturon]

Nominating.

[DanielKeep]

I believe this to be a problem specifically with clone for &'a T. Take the following:

extern crate serialize;
extern crate test;

#[deriving(Encodable)]
pub struct JsonSerialized  {
    really_long_field_name: uint,
}

pub struct MyServer;

pub const ALT_BEHAVIOUR: bool = false;

impl MyServer {
    fn get_json_str(&self) -> &str {
        if ! ALT_BEHAVIOUR {
            serialize::json::encode(&JsonSerialized {
                really_long_field_name: 0,}).as_slice().clone()
        } else {
            let s0 = serialize::json::encode(&JsonSerialized { really_long_field_name: 0, });
            println!("s0: `{}`", s0);
            let s1 = s0.as_slice();
            println!("s1: `{}`", s1);
            let s2 = s1.clone();
            println!("s2: `{}`", s2);
            s2
        }
    }
}

fn mess_the_heap() {
    let s = "ABCDEFGHIJKLMNOPQRSTUVWXYZ".into_string();
    test::black_box(s);
}

fn main() {
    let srv = MyServer;
    let s = srv.get_json_str();
    mess_the_heap();
    println!("s: `{}`", s);
}

Running on Win x86, with rustc 0.13.0-nightly (fac5a0767 2014-11-26 22:37:06 +0000), with ALT_BEHAVIOUR = false, produces the following output:

task '<main>' panicked at 'failed printing to stdout: text was not valid unicode', C:\bot\slave\nightly-win-32\build\src\libstd\io\stdio.rs:229

Running with ALT_BEHAVIOUR = true produces the following output:

s0: `{"really_long_field_name":0}`
s1: `{"really_long_field_name":0}`
s2: `{"really_long_field_name":0}`
s: `ABCDEFGHIJKLMNOPQRSTUVWXYZ0}`

This suggests that the allocation behind the temporary string is being freed and then re-used, despite still holding a borrowed reference.

Note: this code does not pass borrow checking on rustc 0.13.0-dev (8ed288edb 2014-11-06 16:52:09 +0000) i.e. the version used by the playpen. There, the following error occurs:

<anon>:16:17: 17:49 error: borrowed value does not live long enough
<anon>:16                 serialize::json::encode(&JsonSerialized {
<anon>:17                     really_long_field_name: 0,}).as_slice().clone()
<anon>:14:40: 27:10 note: reference must be valid for the anonymous lifetime #1 defined on the block at 14:39...
<anon>:14         fn get_json_str(&self) -> &str {
<anon>:15             if ! ALT_BEHAVIOUR {
<anon>:16                 serialize::json::encode(&JsonSerialized {
<anon>:17                     really_long_field_name: 0,}).as_slice().clone()
<anon>:18             } else {
<anon>:19                 let s0 = serialize::json::encode(&JsonSerialized { really_long_field_name: 0, });
          ...
<anon>:15:32: 18:14 note: ...but borrowed value is only valid for the block at 15:31
<anon>:15             if ! ALT_BEHAVIOUR {
<anon>:16                 serialize::json::encode(&JsonSerialized {
<anon>:17                     really_long_field_name: 0,}).as_slice().clone()
<anon>:18             } else {
<anon>:21:26: 21:28 error: `s0` does not live long enough
<anon>:21                 let s1 = s0.as_slice();
                                   ^~
<anon>:14:40: 27:10 note: reference must be valid for the anonymous lifetime #1 defined on the block at 14:39...
<anon>:14         fn get_json_str(&self) -> &str {
<anon>:15             if ! ALT_BEHAVIOUR {
<anon>:16                 serialize::json::encode(&JsonSerialized {
<anon>:17                     really_long_field_name: 0,}).as_slice().clone()
<anon>:18             } else {
<anon>:19                 let s0 = serialize::json::encode(&JsonSerialized { really_long_field_name: 0, });
          ...
<anon>:18:20: 26:14 note: ...but borrowed value is only valid for the block at 18:19
<anon>:18             } else {
<anon>:19                 let s0 = serialize::json::encode(&JsonSerialized { really_long_field_name: 0, });
<anon>:20                 println!("s0: `{}`", s0);
<anon>:21                 let s1 = s0.as_slice();
<anon>:22                 println!("s1: `{}`", s1);
<anon>:23                 let s2 = s1.clone();
          ...
error: aborting due to 2 previous errors
playpen: application terminated with error code 101

Which suggests a recent regression.

This particular case was found by rictic on IRC.

I should also note that I was incredibly lucky with this: making the "ABCDEF..." string constant longer or shorter causes the problem to disappear!

[ftxqxd]

A smaller form of the same bug:

fn staticify<T>(x: &T) -> &'static T {
    x.clone()
}

This is obviously not safe, and allows one to leak references into proc(): Sends. Strangely, I can’t seem to replicate this issue using the UFCS Clone::clone(x) form, no matter what *s or &s I attach to x—the issue seems to only be present for normal method-style calls.

(This was discovered by tamird and bluss on IRC.)

[tamird]

cc @glennpratt

[Aatch]

This is probably not specific to static lifetimes, as this passes:

fn leak<'a, T>(x: T) -> &'a T {
    (&x).clone()
}

I'm returning a reference to what is essentially a local variable! Not good, not good at all.

[kmcallister]

git bisect pins this on 336349c which was #18694.

[kmcallister]

cc @nikomatsakis