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

[BUG] Closure state is stack allocated #217

Closed
DayDun opened this issue May 20, 2023 · 10 comments
Closed

[BUG] Closure state is stack allocated #217

DayDun opened this issue May 20, 2023 · 10 comments
Assignees
Labels
bug Something isn't working mojo Issues that are related to mojo mojo-lang Tag for all issues related to language. mojo-repo Tag all issues with this label

Comments

@DayDun
Copy link

DayDun commented May 20, 2023

Bug Description

fn f(func: fn() -> None) -> fn() capturing -> None:
    fn wrapper():
        func()
    return wrapper

fn g():
    pass

f(g)()

crashes with

error: Execution was interrupted, reason: signal SIGSEGV: address not mapped to object (fault address: 0x0).
The process has been left at the point where it was interrupted, use "thread return -x" to return to the state before expression evaluation.

Expected behavior would be for it to not crash.

Note that making func a parameter instead does work

fn f[func: fn() -> None]() -> fn() capturing -> None:
    fn wrapper():
        func()
    return wrapper

fn g():
    pass

f[g]()()

Steps to Reproduce

Run the provided code.

Context

11:devices:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-poddc6801ca_838f_451d_8258_ad4fd3f41a6d.slice/cri-containerd-64526640c2bf917695ffc3502241083d511fa40e64ed25700e17bfdeb28e172c.scope
10:memory:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-poddc6801ca_838f_451d_8258_ad4fd3f41a6d.slice/cri-containerd-64526640c2bf917695ffc3502241083d511fa40e64ed25700e17bfdeb28e172c.scope
9:cpuset:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-poddc6801ca_838f_451d_8258_ad4fd3f41a6d.slice/cri-containerd-64526640c2bf917695ffc3502241083d511fa40e64ed25700e17bfdeb28e172c.scope
8:freezer:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-poddc6801ca_838f_451d_8258_ad4fd3f41a6d.slice/cri-containerd-64526640c2bf917695ffc3502241083d511fa40e64ed25700e17bfdeb28e172c.scope
7:net_cls,net_prio:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-poddc6801ca_838f_451d_8258_ad4fd3f41a6d.slice/cri-containerd-64526640c2bf917695ffc3502241083d511fa40e64ed25700e17bfdeb28e172c.scope
6:cpu,cpuacct:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-poddc6801ca_838f_451d_8258_ad4fd3f41a6d.slice/cri-containerd-64526640c2bf917695ffc3502241083d511fa40e64ed25700e17bfdeb28e172c.scope
5:pids:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-poddc6801ca_838f_451d_8258_ad4fd3f41a6d.slice/cri-containerd-64526640c2bf917695ffc3502241083d511fa40e64ed25700e17bfdeb28e172c.scope
4:blkio:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-poddc6801ca_838f_451d_8258_ad4fd3f41a6d.slice/cri-containerd-64526640c2bf917695ffc3502241083d511fa40e64ed25700e17bfdeb28e172c.scope
3:hugetlb:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-poddc6801ca_838f_451d_8258_ad4fd3f41a6d.slice/cri-containerd-64526640c2bf917695ffc3502241083d511fa40e64ed25700e17bfdeb28e172c.scope
2:perf_event:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-poddc6801ca_838f_451d_8258_ad4fd3f41a6d.slice/cri-containerd-64526640c2bf917695ffc3502241083d511fa40e64ed25700e17bfdeb28e172c.scope
1:name=systemd:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-poddc6801ca_838f_451d_8258_ad4fd3f41a6d.slice/cri-containerd-64526640c2bf917695ffc3502241083d511fa40e64ed25700e17bfdeb28e172c.scope
0::/
@DayDun DayDun added the bug Something isn't working label May 20, 2023
@Mogball
Copy link

Mogball commented May 20, 2023

This is a known issue. Capture state is stack allocated. It should be heap allocated and it's something we plan to fix.

@Mogball Mogball changed the title [BUG] Closure crash [BUG] Closure state is stack allocated May 20, 2023
@Mogball
Copy link

Mogball commented May 22, 2023

This issue is observed here as well #111

@survuvi
Copy link

survuvi commented May 24, 2023

With the magic %%python the output of this closure is ok: x = 7, y = 4, z = 2, return: 13

%%python
def outer_func(x):
    y = 4
    def inner_func(z):
        print(f'x = {x}, y = {y}, z = {z}')
        return x + y + z
    return inner_func

closure = outer_func(7)
print(closure(2))  

But without:

def outer_func(x):
    y = 4
    def inner_func(z):
        print("x = ", x, " y = ",  y, " z = ", z)
        return x + y + z
    return inner_func

closure = outer_func(7)
print(closure(2))

_error: Expression [5]:10:14: no matching function in call to 'print':
print("x = ", x, " y = ", y, " z = ",z)
~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/.modular/Kernels/mojo/Stdlib/IO.mojo:244:1: candidate not viable: callee expects 0 arguments, but 6 were specified
fn print():
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:249:1: candidate not viable: callee expects 1 argument, but 6 were specified
fn print(t: DType):
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:258:1: candidate not viable: callee expects 1 argument, but 6 were specified
fn print(x: String):
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:268:1: candidate not viable: callee expects 1 argument, but 6 were specified
fn print(x: StringRef):
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:278:1: candidate not viable: callee expects 1 argument, but 6 were specified
fn print(x: StringLiteral):
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:287:1: candidate not viable: callee expects 1 argument, but 6 were specified
fn print(x: Bool):
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:296:1: candidate not viable: callee expects 1 argument, but 6 were specified
fn print(x: FloatLiteral):
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:305:1: candidate not viable: callee expects 1 argument, but 6 were specified
fn print(x: Int):
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:314:1: candidate not viable: callee expects 2 input parameters but 0 were provided
fn print[
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:339:1: candidate not viable: callee expects 1 input parameter but 0 were provided
fn print[type: DType](x: Atomic[type]):
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:352:1: candidate not viable: callee expects 1 input parameter but 0 were provided
fn print[length: Int](shape: DimList):
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:378:1: candidate not viable: callee expects 1 argument, but 6 were specified
fn print(obj: object):
^

/.modular/Kernels/mojo/Stdlib/IO.mojo:397:1: candidate not viable: argument #1 cannot be converted from 'object' to '_Printable'
fn print(*elements: _Printable):
^

error: Expression [5]:12:12: 'fn(object) raises capturing -> object' value cannot be converted to 'object' in return value
return inner_func_

@Manish0045
Copy link

from time import time
def greet():
i=0
for i in range(10):
time.sleep(1)
print(i)
print('Hello world!')
greet()
/ I got same error in the above code can anyone suggest what am I doing wrong in it.

@oskgo
Copy link

oskgo commented Aug 30, 2023

The following variant segfaults in the beta:

fn f(func: fn()-> None) -> fn() capturing -> None:
        fn wrapper():
            func()
        return wrapper

fn g():
    pass

def main():
    f(g)()

@ematejska ematejska added mojo Issues that are related to mojo mojo-stdlib Tag for issues related to standard library labels Sep 7, 2023
@ematejska
Copy link
Collaborator

@Mogball Stdlib or lang for labels?

@Mogball
Copy link

Mogball commented Sep 7, 2023

lang. Closures are a language issue

@ematejska ematejska added mojo-lang Tag for all issues related to language. and removed mojo-stdlib Tag for issues related to standard library labels Sep 8, 2023
@emillma
Copy link

emillma commented Sep 15, 2023

The following examples might be relevant.

fn outer(number: Int = 1) -> fn () capturing -> None:
    fn inner():
        print("inner=", number)  #prints  inner=94467979384400 (varies)
    print("outer=", number)  #prints  outer=1
    return inner
fn outer(number: Int = 1) -> fn () capturing -> None:
    print("outer=", number)  #prints  inner=1
    fn inner():
        print("inner=", number)  #prints inner=1
    print("outer=", number)  #prints  outer=1
    return inner
fn outer(number: Int = 1) -> fn () capturing -> None:
    print("outer=", number)  #SEGFAULT
    fn inner():
        print("inner=", number)  #SEGFAULT
    return inner

@oskgo
Copy link

oskgo commented Sep 15, 2023

@emillma Reproducing this behavior in the playground requires using print(number) rather than print("outer=", number)

@Brian-M-J
Copy link
Contributor

I started a discussion (#1690) on why stack allocated closures may not necessarily be a "bug" to "fix". Tell me what you guys think.

@ematejska ematejska added the mojo-repo Tag all issues with this label label May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working mojo Issues that are related to mojo mojo-lang Tag for all issues related to language. mojo-repo Tag all issues with this label
Projects
None yet
Development

No branches or pull requests

9 participants