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

closure in record triggers a memory leak and abort #1896

Closed
erickt opened this issue Feb 24, 2012 · 15 comments
Closed

closure in record triggers a memory leak and abort #1896

erickt opened this issue Feb 24, 2012 · 15 comments
Assignees
Labels
A-codegen Area: Code generation I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.

Comments

@erickt
Copy link
Contributor

erickt commented Feb 24, 2012

There seems to be an issue with closures in records and the type checker. This code segfaults with a leaked memory message:

type t<T> = { f: fn() -> T };

fn f<T>(_x: t<T>) {}

fn main() {
    let x: t<()> = { f: { || () } };
    f(x);
}

However, if you remove : t<()> from let x: t<()> = { f: { || () } };, it works just fine.

@nikomatsakis
Copy link
Contributor

I am running into some leaks myself which look similar. I will assign to myself and hope that fixing my problems will fix this one too.

@ghost ghost assigned nikomatsakis Feb 24, 2012
@nikomatsakis
Copy link
Contributor

Actually, I am not able to reproduce this problem.

@brson
Copy link
Contributor

brson commented Feb 24, 2012

I do see it. Really surprising.

@brson
Copy link
Contributor

brson commented Feb 24, 2012

When you rewrite it like this (with fn@) it works:

type t<T> = { f: fn@() -> T };                                                                                                                                                

fn f<T>(_x: t<T>) {}                                                                                                                                                          

fn main() {                                                                                                                                                                   
    let x: t<()> = { f: { || () } };                                                                                                                                          
    f(x);                                                                                                                                                                     
}      

I'm not actually sure what fn means in this record.

@nikomatsakis
Copy link
Contributor

So, it would not be possible to store an fn& into the record... the type of the record says that it contains some kind of closure. Our restrictions on fn& should ensure that this closure is either an fn@ or fn~. The closure kind inferencer will pick @ in the absence of any other good choice, so {f: {|| ()}} should have the type {f: fn@() -> ()}.

@nikomatsakis
Copy link
Contributor

Revisiting this, it's clear why there is a leak: an fn() type should not appear in a record. The drop glue will take no action.

@catamorphism
Copy link
Contributor

This issue appears to be fixed: the typechecker now rejects this program. Added it as a test case in 2a53640 .

@catamorphism
Copy link
Contributor

Another test case:

use std;

type boxedFn = { theFn: fn () -> uint };

fn createClosure (closedUint: uint) -> boxedFn {
    { theFn: fn@ () -> uint { closedUint } }
}

#[test]
fn testForLeakage () {
    let aFn: boxedFn = createClosure(10);

    let myInt: uint = aFn.theFn();

    assert myInt == 10;
}

@nikomatsakis
Copy link
Contributor

I think this will get fixed as part of the regions work on fns I need to do.

@nikomatsakis
Copy link
Contributor

#2202 specifically.

@bblum
Copy link
Contributor

bblum commented Jul 12, 2012

This also leaks; is it the same issue?

fn zipWith<A:copy,B:copy,C>(f: fn@((A,B))->C) -> fn(+~[A]) -> fn(+~[B]) -> ~[C] {
    fn@(+a: ~[A]) -> fn(+b: ~[B]) -> ~[C] {
        fn@(+b: ~[B], move a) -> ~[C] {
            vec::zip(a,b).map(f)
        }
    }
}

fn sum(x: (int,int)) -> int { let (y,z) = x; y+z }

fn main() {
    for zipWith(sum)(~[1,2,3,4])(~[10,20,30,40]).each |i| {
        io::println(#fmt("%d", i));
    }
}

It prints 11, 22, 33, 44, then proclaims:

Unreclaimed object found at 0x7fe6241009a0: (~[1, 2, 3, 4], fn)
Unreclaimed object found at 0x7fe624100900: (fn)
leaked memory in rust main loop (2 objects)

@nikomatsakis
Copy link
Contributor

same issue.

@bblum
Copy link
Contributor

bblum commented Jul 16, 2012

fn add(n: int) -> fn@(int) -> int {
      fn@(m: int) -> int { m + n }
}

fn main()
{
      assert add(3)(4) == 7;
        let add3 : fn(int)->int = add(3);
          assert add3(4) == 7;
}

@jruderman
Copy link
Contributor

fn add(n: int) -> fn@(int) -> int {
    fn@(m: int) -> int { m + n }
}

fn main()
{
    // Does not leak
    assert add(3)(4) == 7;

    // Does not leak
    let add1 : fn@(int)->int = add(1);
    assert add1(6) == 7;

    // Does not leak
    let add2 : &(fn@(int)->int) = &add(2);
    assert (*add2)(5) == 7;

    // Leaks
    let add3 : fn(int)->int = add(3);
    assert add3(4) == 7;
}

catamorphism added a commit that referenced this issue Nov 1, 2012
@catamorphism
Copy link
Contributor

This appears to be fixed (@bblum's example with zipWith doesn't compile now due to borrowed pointer errors, but I couldn't figure out how to get a working version in short order, and the other tests all work). Added test cases; closing.

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Nov 6, 2012
Fixes rust-lang#1896 which was never truly fixed, just masked.
The given tests would have failed had they used `~fn()` and
not `@fn()`.  They now result in compilation errors.

Fixes rust-lang#2978.

Necessary first step for rust-lang#2202, rust-lang#2263.
nikomatsakis added a commit that referenced this issue Nov 6, 2012
Fixes #1896 which was never truly fixed, just masked.
The given tests would have failed had they used `~fn()` and
not `@fn()`.  They now result in compilation errors.

Fixes #2978.

Necessary first step for #2202, #2263.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

No branches or pull requests

6 participants