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

Missing cleanup on escape from functional record update #21486

Closed
arielb1 opened this issue Jan 21, 2015 · 6 comments
Closed

Missing cleanup on escape from functional record update #21486

arielb1 opened this issue Jan 21, 2015 · 6 comments
Assignees
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. P-medium Medium priority
Milestone

Comments

@arielb1
Copy link
Contributor

arielb1 commented Jan 21, 2015

I was playing around with #21407, and I think I found a distinct bug in FRU – if a field initialiser escapes (panics/returns/breaks), some of the fields in the original struct get leaked.

For example:

struct Noisy(u32);
impl Drop for Noisy {
    fn drop(&mut self) {
        println!("splat #{}!", self.0);
    }
}

struct Foo { n0: Noisy, n1: Noisy }

fn leak() -> Foo {
    let old_foo = Foo { n0: Noisy(0), n1: Noisy(1) };
    Foo { n0: { return Foo { n0: Noisy(3), n1: Noisy(4) } }, ..old_foo } ;
}

fn main() {
    drop(leak());
}

This prints

splat #0!
splat #3!
splat #4!

Observe that the Noisy(1) is never destroyed.

@pnkfelix
Copy link
Member

nominating, suggest 1.0 polish, P-high.

@pnkfelix
Copy link
Member

P-high, 1.0 polish.

@pnkfelix pnkfelix added this to the 1.0 milestone Jan 29, 2015
@pnkfelix pnkfelix added P-medium Medium priority and removed I-nominated labels Jan 29, 2015
@pnkfelix pnkfelix self-assigned this Apr 2, 2015
@pnkfelix
Copy link
Member

This seems to have been fixed.

@pnkfelix
Copy link
Member

i wonder if #23201 is what fixed it ... (it certainly seems like it might also have been injected by #23112 ).

@pnkfelix pnkfelix added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Apr 10, 2015
@pnkfelix
Copy link
Member

in any case, seems like the remaining work here is to add a regression test.

@pnkfelix
Copy link
Member

(but, ooooh, there are definitely other bugs lying around in this area.... will file some tickets soonish.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

2 participants