Skip to content

Grumpy panics when it can't fulfill an allocation #28

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

Open
alanjds opened this issue Aug 21, 2018 · 6 comments
Open

Grumpy panics when it can't fulfill an allocation #28

alanjds opened this issue Aug 21, 2018 · 6 comments
Labels
imported Imported from google/grumpy

Comments

@alanjds
Copy link

alanjds commented Aug 21, 2018

google#166 opened by @S-YOU on 19 Jan 2017

This python code

x = [True] * sys.maxint

cause

resultElems := make([]*Object, newNumElems)

in seq.go to be panicked. I can't think of a way to handle that.

@alanjds
Copy link
Author

alanjds commented Aug 21, 2018

Comment by trotterdylan
Thursday Jan 19, 2017 at 15:23 GMT


I think CPython would raise MemoryError in this case but I'm not sure that we can do the same here: there doesn't appear to be a way to catch failed memory allocations in Go.

@alanjds
Copy link
Author

alanjds commented Aug 21, 2018

Comment by meadori
Friday Jan 27, 2017 at 15:22 GMT


CPython does raise a memory error, but not by catching a failed malloc. The check in list_repeat is preemptive:

if (n > 0 && Py_SIZE(a) > PY_SSIZE_T_MAX / n)
        return PyErr_NoMemory();

We can have similar checks in Grumpy.

@alanjds
Copy link
Author

alanjds commented Aug 21, 2018

Comment by S-YOU
Friday Jan 27, 2017 at 15:32 GMT


x = [True] * (sys.maxint / 1000000) still panic, so PY_SSIZE_T_MAX is not a ideal solution I guess.

@alanjds
Copy link
Author

alanjds commented Aug 21, 2018

Comment by meadori
Friday Jan 27, 2017 at 16:01 GMT


Hmmm, so the panic is from makeslice: panic: runtime error: makeslice: len out of range.
We attempt to check for this in Grumpy. However, the check is different than that of the check used for slices in the Go runtime. Maybe we should implement a check more similar to the Go one?

@alanjds
Copy link
Author

alanjds commented Aug 21, 2018

Comment by trotterdylan
Friday Jan 27, 2017 at 17:38 GMT


Oh that's interesting. So CPython does not raise MemoryError when malloc() fails?

@alanjds
Copy link
Author

alanjds commented Aug 21, 2018

Comment by meadori
Friday Jan 27, 2017 at 18:10 GMT


It does raise MemoryErrors for failed allocation too. For the list repeat case, however, it will try the check I posted in a previous comment before even attempting to allocate memory. CPython does these kinds of preemptive checks in other places too.

Go also does checks like these, hence the panic from makeslice. I think we should model our check off the makeslice one and create a MemoryError if it fails.

@alanjds alanjds added the imported Imported from google/grumpy label Aug 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imported Imported from google/grumpy
Projects
None yet
Development

No branches or pull requests

1 participant