Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpectedly long lifetime of values created in let bindings #98052

Open
theduke opened this issue Jun 13, 2022 · 5 comments
Open

Unexpectedly long lifetime of values created in let bindings #98052

theduke opened this issue Jun 13, 2022 · 5 comments
Labels
A-control-flow Area: Control flow C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@theduke
Copy link
Contributor

theduke commented Jun 13, 2022

Today I stumbled over an unexpected deadlock related to lifetime scoping of values inside a let binding.

It seems like values created in a let binding are kept alive until the end of the inner block, even if the value is surrounded by a block.

I'm not sure if this is the expected behaviour, but it is counter to my expectation of "values live until the end of the surrounding block".

To illustrate:

use std::sync::Mutex;

fn main() {
    let m = Mutex::new(Some(true));
    test_mutex(&m)
}

fn test_mutex(m: &Mutex<Option<bool>>) {
    if let Some(x) = { (*m.lock().unwrap()).clone() } {
        //             ^^^^^^^^^^^^^^^^^^^^^^^^^
        //             Mutex is locked, inner value is cloned.

        // Ensure x is actually bool, not a reference.
        let x: bool = x;
        // do expensive work...
        *m.lock().unwrap() = Some(!x);
        // ^^^^^^
        // DEADLOCK!
    }
}

My expectation was that that the lock would go out of scope at the beginning of the inner block, but it does not and is kept alive until the inner block ends, leading to the deadlock - as can be verified by looking at the MIR:

MIR Output
// WARNING: This output format is intended for human consumers only
// and is subject to change without notice. Knock yourself out.
fn main() -> () {
    let mut _0: ();                      // return place in scope 0 at [src/main.rs:3:11: 3:11](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    let _1: std::sync::Mutex>; // in scope 0 at [src/main.rs:4:9: 4:10](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    let mut _2: std::option::Option; // in scope 0 at [src/main.rs:4:24: 4:34](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    let mut _3: &std::sync::Mutex>; // in scope 0 at [src/main.rs:5:16: 5:18](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    let _4: &std::sync::Mutex>; // in scope 0 at [src/main.rs:5:16: 5:18](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    scope 1 {
        debug m => _1;                   // in scope 1 at [src/main.rs:4:9: 4:10](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    }
bb0: {
    Deinit(_2);                      // scope 0 at [src/main.rs:4:24: 4:34](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    ((_2 as Some).0: bool) = const true; // scope 0 at [src/main.rs:4:24: 4:34](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    discriminant(_2) = 1;            // scope 0 at [src/main.rs:4:24: 4:34](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _1 = Mutex::<Option<bool>>::new(move _2) -> bb1; // scope 0 at [src/main.rs:4:13: 4:35](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // mir::Constant
                                     // + span: [src/main.rs:4:13: 4:23](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // + user_ty: UserType(0)
                                     // + literal: Const { ty: fn(Option<bool>) -> Mutex<Option<bool>> {Mutex::<Option<bool>>::new}, val: Value(Scalar(<ZST>)) }
}

bb1: {
    _4 = &_1;                        // scope 1 at [src/main.rs:5:16: 5:18](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _3 = _4;                         // scope 1 at [src/main.rs:5:16: 5:18](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _0 = test_mutex(move _3) -> bb2; // scope 1 at [src/main.rs:5:5: 5:19](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // mir::Constant
                                     // + span: [src/main.rs:5:5: 5:15](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // + literal: Const { ty: for<'r> fn(&'r Mutex<Option<bool>>) {test_mutex}, val: Value(Scalar(<ZST>)) }
}

bb2: {
    return;                          // scope 0 at [src/main.rs:6:2: 6:2](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
}

}

fn test_mutex(_1: &Mutex<Option>) -> () {
debug m => _1; // in scope 0 at src/main.rs:8:15: 8:16
let mut _0: (); // return place in scope 0 at src/main.rs:8:40: 8:40
let mut _2: std::option::Option; // in scope 0 at src/main.rs:9:22: 9:70
let _3: std::option::Option; // in scope 0 at src/main.rs:9:28: 9:33
let mut _4: &std::option::Option; // in scope 0 at src/main.rs:9:36: 9:61
let _5: &std::option::Option; // in scope 0 at src/main.rs:9:36: 9:61
let mut _6: &std::sync::MutexGuard<std::option::Option>; // in scope 0 at src/main.rs:9:36: 9:61
let _7: std::sync::MutexGuard<std::option::Option>; // in scope 0 at src/main.rs:9:36: 9:53
let mut _8: std::result::Result<std::sync::MutexGuard<std::option::Option>, std::sync::PoisonError<std::sync::MutexGuard<std::option::Option>>>; // in scope 0 at src/main.rs:9:36: 9:44
let mut _9: &std::sync::Mutex<std::option::Option>; // in scope 0 at src/main.rs:9:36: 9:44
let mut _10: isize; // in scope 0 at src/main.rs:9:12: 9:19
let _11: bool; // in scope 0 at src/main.rs:9:17: 9:18
let _12: bool; // in scope 0 at src/main.rs:14:13: 14:14
let mut _13: std::option::Option; // in scope 0 at src/main.rs:16:30: 16:38
let mut _14: bool; // in scope 0 at src/main.rs:16:35: 16:37
let mut _15: bool; // in scope 0 at src/main.rs:16:36: 16:37
let mut _16: &mut std::option::Option; // in scope 0 at src/main.rs:16:9: 16:27
let mut _17: &mut std::sync::MutexGuard<std::option::Option>; // in scope 0 at src/main.rs:16:10: 16:27
let mut _18: std::sync::MutexGuard<std::option::Option>; // in scope 0 at src/main.rs:16:10: 16:27
let mut _19: std::result::Result<std::sync::MutexGuard<std::option::Option>, std::sync::PoisonError<std::sync::MutexGuard<std::option::Option>>>; // in scope 0 at src/main.rs:16:10: 16:18
let mut _20: &std::sync::Mutex<std::option::Option>; // in scope 0 at src/main.rs:16:10: 16:18
scope 1 {
debug value => _3; // in scope 1 at src/main.rs:9:28: 9:33
}
scope 2 {
debug x => _11; // in scope 2 at src/main.rs:9:17: 9:18
}
scope 3 {
debug x => _12; // in scope 3 at src/main.rs:14:13: 14:14
}

bb0: {
    _9 = _1;                         // scope 0 at [src/main.rs:9:36: 9:44](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _8 = Mutex::<Option<bool>>::lock(move _9) -> bb1; // scope 0 at [src/main.rs:9:36: 9:44](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // mir::Constant
                                     // + span: [src/main.rs:9:38: 9:42](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // + literal: Const { ty: for<'r> fn(&'r Mutex<Option<bool>>) -> Result<MutexGuard<'r, Option<bool>>, PoisonError<MutexGuard<'r, Option<bool>>>> {Mutex::<Option<bool>>::lock}, val: Value(Scalar(<ZST>)) }
}

bb1: {
    _7 = Result::<MutexGuard<Option<bool>>, PoisonError<MutexGuard<Option<bool>>>>::unwrap(move _8) -> bb2; // scope 0 at [src/main.rs:9:36: 9:53](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // mir::Constant
                                     // + span: [src/main.rs:9:45: 9:51](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // + literal: Const { ty: fn(Result<MutexGuard<Option<bool>>, PoisonError<MutexGuard<Option<bool>>>>) -> MutexGuard<Option<bool>> {Result::<MutexGuard<Option<bool>>, PoisonError<MutexGuard<Option<bool>>>>::unwrap}, val: Value(Scalar(<ZST>)) }
}

bb2: {
    _6 = &_7;                        // scope 0 at [src/main.rs:9:36: 9:61](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _5 = <MutexGuard<Option<bool>> as Deref>::deref(move _6) -> [return: bb3, unwind: bb12]; // scope 0 at [src/main.rs:9:36: 9:61](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // mir::Constant
                                     // + span: [src/main.rs:9:36: 9:61](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // + literal: Const { ty: for<'r> fn(&'r MutexGuard<Option<bool>>) -> &'r <MutexGuard<Option<bool>> as Deref>::Target {<MutexGuard<Option<bool>> as Deref>::deref}, val: Value(Scalar(<ZST>)) }
}

bb3: {
    _4 = _5;                         // scope 0 at [src/main.rs:9:36: 9:61](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _3 = <Option<bool> as Clone>::clone(move _4) -> [return: bb4, unwind: bb12]; // scope 0 at [src/main.rs:9:36: 9:61](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // mir::Constant
                                     // + span: [src/main.rs:9:54: 9:59](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // + literal: Const { ty: for<'r> fn(&'r Option<bool>) -> Option<bool> {<Option<bool> as Clone>::clone}, val: Value(Scalar(<ZST>)) }
}

bb4: {
    drop(_7) -> bb5;                 // scope 0 at [src/main.rs:9:61: 9:62](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
}

bb5: {
    _2 = _3;                         // scope 1 at [src/main.rs:9:63: 9:68](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _10 = discriminant(_2);          // scope 0 at [src/main.rs:9:12: 9:19](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    switchInt(move _10) -> [1_isize: bb6, otherwise: bb10]; // scope 0 at [src/main.rs:9:12: 9:19](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
}

bb6: {
    _11 = ((_2 as Some).0: bool);    // scope 0 at [src/main.rs:9:17: 9:18](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _12 = _11;                       // scope 0 at [src/main.rs:14:23: 14:24](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _15 = _12;                       // scope 3 at [src/main.rs:16:36: 16:37](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _14 = Not(move _15);             // scope 3 at [src/main.rs:16:35: 16:37](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    Deinit(_13);                     // scope 3 at [src/main.rs:16:30: 16:38](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    ((_13 as Some).0: bool) = move _14; // scope 3 at [src/main.rs:16:30: 16:38](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    discriminant(_13) = 1;           // scope 3 at [src/main.rs:16:30: 16:38](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _20 = _1;                        // scope 3 at [src/main.rs:16:10: 16:18](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _19 = Mutex::<Option<bool>>::lock(move _20) -> bb7; // scope 3 at [src/main.rs:16:10: 16:18](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // mir::Constant
                                     // + span: [src/main.rs:16:12: 16:16](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // + literal: Const { ty: for<'r> fn(&'r Mutex<Option<bool>>) -> Result<MutexGuard<'r, Option<bool>>, PoisonError<MutexGuard<'r, Option<bool>>>> {Mutex::<Option<bool>>::lock}, val: Value(Scalar(<ZST>)) }
}

bb7: {
    _18 = Result::<MutexGuard<Option<bool>>, PoisonError<MutexGuard<Option<bool>>>>::unwrap(move _19) -> bb8; // scope 3 at [src/main.rs:16:10: 16:27](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // mir::Constant
                                     // + span: [src/main.rs:16:19: 16:25](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // + literal: Const { ty: fn(Result<MutexGuard<Option<bool>>, PoisonError<MutexGuard<Option<bool>>>>) -> MutexGuard<Option<bool>> {Result::<MutexGuard<Option<bool>>, PoisonError<MutexGuard<Option<bool>>>>::unwrap}, val: Value(Scalar(<ZST>)) }
}

bb8: {
    _17 = &mut _18;                  // scope 3 at [src/main.rs:16:10: 16:27](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    _16 = <MutexGuard<Option<bool>> as DerefMut>::deref_mut(move _17) -> [return: bb9, unwind: bb11]; // scope 3 at [src/main.rs:16:9: 16:27](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // mir::Constant
                                     // + span: [src/main.rs:16:9: 16:27](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                     // + literal: Const { ty: for<'r> fn(&'r mut MutexGuard<Option<bool>>) -> &'r mut <MutexGuard<Option<bool>> as Deref>::Target {<MutexGuard<Option<bool>> as DerefMut>::deref_mut}, val: Value(Scalar(<ZST>)) }
}

bb9: {
    (*_16) = move _13;               // scope 3 at [src/main.rs:16:9: 16:38](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    drop(_18) -> bb10;               // scope 3 at [src/main.rs:16:38: 16:39](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
}

bb10: {
    return;                          // scope 0 at [src/main.rs:20:2: 20:2](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
}

bb11 (cleanup): {
    drop(_18) -> bb13;               // scope 3 at [src/main.rs:16:38: 16:39](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
}

bb12 (cleanup): {
    drop(_7) -> bb13;                // scope 0 at [src/main.rs:9:61: 9:62](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
}

bb13 (cleanup): {
    resume;                          // scope 0 at [src/main.rs:8:1: 20:2](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
}

}

In contrast, this code works as expected: if let Some(x) = { let value = m.lock().unwrap().clone(); value } {

This seems very counter-intuitive if your assumption is that block = lifetime scope.

Maybe it's not treated as a block because it's a single expression?

rustc 1.63.0-nightly (e09449220 2022-05-31)

@eggyal
Copy link
Contributor

eggyal commented Jun 13, 2022

Maybe it's not treated as a block because it's a single expression?

Yeah, I think that because the expression assigned to x must live to the end of the if let, any scopes of its subexpressions are not terminated any earlier (even though they could be).

@rustbot label +A-control-flow +C-enhancement +T-compiler

@rustbot rustbot added A-control-flow Area: Control flow C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 13, 2022
@eggyal
Copy link
Contributor

eggyal commented Jun 13, 2022

The Rust Reference on Temporaries states:

The drop scope of the temporary is usually the end of the enclosing statement.

In this case, when the MutexGuard is part of the final expression in the block, the "enclosing statement" is the if let.

@eggyal
Copy link
Contributor

eggyal commented Jun 13, 2022

@Neutron3529
Copy link
Contributor

Neutron3529 commented Jun 13, 2022

The Rust Reference on Temporaries states:

The drop scope of the temporary is usually the end of the enclosing statement.

In this case, when the MutexGuard is part of the final expression in the block, the "enclosing statement" is the if let.

That's not always true, since

if x.borrow().is_negative(){ *x.borrow_mut() = 0 }else{*x.borrow_mut() = 0}

does not painc. Since x.borrow().is_negative() is not a statement, its "enclosing statement" is the if, it should not drop as that early.

@eggyal
Copy link
Contributor

eggyal commented Jun 13, 2022

@Neutron3529 you're right, of course, but the lack of a binding clearly makes a difference; without one (as in your example), there cannot be any dependency on the temporary whereas with one there could be.

I unearthed a couple of declined pre-1.0 RFCs: Static drop semantics and Allow Eager Drops, which from a cursory glance appear to have been attempts at solving this (and related) issues. Perhaps worth reviewing those and the associated discussions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-control-flow Area: Control flow C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants