Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

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

Open
S-YOU opened this issue Jan 19, 2017 · 6 comments
Open

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

S-YOU opened this issue Jan 19, 2017 · 6 comments

Comments

@S-YOU
Copy link
Contributor

S-YOU commented Jan 19, 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.

@trotterdylan
Copy link
Contributor

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.

@trotterdylan trotterdylan changed the title panic on make on corner case Grumpy panics when it can't fulfill an allocation Jan 19, 2017
@meadori
Copy link
Contributor

meadori commented Jan 27, 2017

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.

@S-YOU
Copy link
Contributor Author

S-YOU commented Jan 27, 2017

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

@meadori
Copy link
Contributor

meadori commented Jan 27, 2017

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?

@trotterdylan
Copy link
Contributor

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

@meadori
Copy link
Contributor

meadori commented Jan 27, 2017

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants