Closed
Description
openedon May 18, 2020
Problem: function is explicitly defined as accepting Box<dyn Fn + 'static>
and yet it accepts Box<dyn Fn + 'a>
where 'a
is definitely less than 'static
.
It allows to keep reference to an object past object lifetime and therefore cause a crash.
Sorry for an extremely long example, but issue in question is very fragile and I couldn't reproduce it with smaller code.
use std::cell::RefCell;
// a simple trait with one function accepting two arguments:
// one by mutable reference and another one by value
trait Foo {
type MutArg; // type of an argument passed by mutable reference
type ValArg; // type of an argument passed by value
fn foo(&self, marg: &mut Self::MutArg, varg: Self::ValArg);
}
// ValArg can be a complex function which can be used
// to pass MutArg from parent to child
// note that all trait objects must have 'static lifetime
pub type FnValArg<MutArg> = Box<dyn
FnOnce(Box<dyn
FnOnce(&mut MutArg) + 'static
>) + 'static
>;
// FnFooBox is boxed Foo which ValArg is a function
type FnFooBox<MutArg1, MutArg2> = Box<dyn
Foo<ValArg=FnValArg<MutArg1>, MutArg=MutArg2>
>;
// FnFoo factory creates boxed FnFoo value on the fly
type FnFooFactory<MutArg1, MutArg2> = Box<dyn
Fn() -> FnFooBox<MutArg1, MutArg2>
>;
// FactoryFoo is a struct storing factory
// and implementing Foo
struct FactoryFoo<MutArg1, MutArg2> {
factory: FnFooFactory<MutArg1, MutArg2>,
// Note: if instead of factory I store `subfoo` directly,
// bug does not reproduce (i.e. I get lifetime error as expected):
// subfoo: FnFooBox<MutArg1, MutArg2>,
}
impl<MutArg1, MutArg2> Foo for FactoryFoo<MutArg1, MutArg2> {
type MutArg = (MutArg1, MutArg2);
type ValArg = i32; // irrelevant
fn foo(&self, marg: &mut Self::MutArg, _varg: Self::ValArg) {
let (marg1, marg2) = marg;
let subfoo = (self.factory)();
subfoo.foo(
marg2,
// `subfoo.foo` requires `varg` of type `FnValArg`
// `FnValArg` defined as boxed closure with 'static lifetime
//
// this closure captures mutable `marg1`, and therefore
// has the same lifetime as `marg`,
// which is obviously less than 'static
//
// and yet it is accepted and everything works,
// which allows `subfoo` to modify `marg1` if desired
//
// Note: if I move this expression into local variable
// bug does not reproduce (i.e. I get lifetime error as expected)
Box::new(move |subfun| subfun(marg1)),
);
}
}
// now let me illustrate that it's possible to cause a crash
// with this lifetime issue:
// idea is to capture reference to marg1 and store it globally
// global variable storing FnValArg, which in turn captures marg1 from FactoryFoo
thread_local! {
static GLOBAL_FN: RefCell<Option<FnValArg<String>>> = RefCell::new(None);
}
// struct implementing Foo and storing varg
// (i.e. closure with captured `&mut marg1` inside)
// in global variable
struct CrashFnFoo {}
impl Foo for CrashFnFoo {
type ValArg = FnValArg<String>;
type MutArg = i32; // irrelevant
fn foo(&self, _marg: &mut Self::MutArg, varg: Self::ValArg) {
// store varg function in global variable
GLOBAL_FN.with(|global_fn| {
global_fn.replace(Some(varg))
});
}
}
fn main() {
let factory = || -> FnFooBox<String, i32> { Box::new(CrashFnFoo{}) };
let factory_foo = FactoryFoo { factory: Box::new(factory) };
{
let mut marg = (String::from("some data"), 0);
// this captures `&mut marg.0` into `GLOBAL_FN`
factory_foo.foo(&mut marg, 0);
}
// by now marg is gone, but reference to it is still
// captured in closure inside of GLOBAL_FN
// now we just have to access it
GLOBAL_FN.with(|global_fn| {
let fn_refcell = RefCell::new(None);
global_fn.swap(&fn_refcell);
if let Some(func) = fn_refcell.into_inner() {
println!("crashing now");
// modifying marg.0 String, which is long dead by now
func(Box::new(|marg1: &mut String| {
marg1.push_str("crash here, please");
}));
}
});
}
I expected this error to happen (and it actually does happen if you change the example even a little bit):
error[E0495]: cannot infer an appropriate lifetime due to conflicting requirements
--> src/lib.rs:45:14
|
45 | let (marg1, marg2) = marg;
| ^^^^^
|
note: first, the lifetime cannot outlive the anonymous lifetime #2 defined on the method body at 44:5...
--> src/lib.rs:44:5
|
44 | / fn foo(&self, marg: &mut Self::MutArg, _varg: Self::ValArg) {
45 | | let (marg1, marg2) = marg;
46 | | let subfoo = (self.factory)();
47 | | self.subfoo.foo(
... |
62 | | );
63 | | }
| |_____^
note: ...so that reference does not outlive borrowed content
--> src/lib.rs:45:14
|
45 | let (marg1, marg2) = marg;
| ^^^^^
= note: but, the lifetime must be valid for the static lifetime...
note: ...so that the expression is assignable
--> src/lib.rs:61:13
|
61 | Box::new(move |subfun| subfun(marg1)),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: expected `std::boxed::Box<(dyn std::ops::FnOnce(std::boxed::Box<(dyn for<'r> std::ops::FnOnce(&'r mut MutArg1) + 'static)>) + 'static)>`
found `std::boxed::Box<dyn std::ops::FnOnce(std::boxed::Box<(dyn for<'r> std::ops::FnOnce(&'r mut MutArg1) + 'static)>)>`
Instead this code is silently accepted and causes a segmentation error when executed.
Meta
I tried it both on stable and nightly rust:
$ rustc --version --verbose
rustc 1.45.0-nightly (7ebd87a7a 2020-05-08)
binary: rustc
commit-hash: 7ebd87a7a1e0e21767422e115c9455ef6e6d4bee
commit-date: 2020-05-08
host: x86_64-pc-windows-msvc
release: 1.45.0-nightly
LLVM version: 9.0
$ rustc --version --verbose
rustc 1.43.0 (4fb7144ed 2020-04-20)
binary: rustc
commit-hash: 4fb7144ed159f94491249e86d5bbd033b5d60550
commit-date: 2020-04-20
host: x86_64-pc-windows-msvc
release: 1.43.0
LLVM version: 9.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Metadata
Assignees
Labels
Area: Lifetimes / regionsArea: Trait systemCategory: This is a bug.Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable ExampleIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessHelping to "clean up" bugs with minimal examples and bisectionsCritical priorityRelevant to the compiler team, which will review and decide on the PR/issue.Performance or correctness regression from one stable version to another.