On Tue, 2021-01-19 at 16:22 +0000, Mark Shannon wrote: > > > On 19/01/2021 3:43 pm, Sebastian Berg wrote: > > On Tue, 2021-01-19 at 13:31 +0000, Mark Shannon wrote: > > > Hi everyone, > > > > > > It's time for yet another PEP :) > > > > > > Fortunately, this one is a small one that doesn't change much. > > > It's aim is to make the VM more robust. > > > > > > Abstract > > > ======== > > > > > > This PEP proposes that machine stack overflow is treated > > > differently > > > from runaway recursion. This would allow programs to set the > > > maximum > > > recursion depth to fit their needs and provide additional safety > > > guarantees. > > > > > > The following program will run safely to completion: > > > > > > sys.setrecursionlimit(1_000_000) > > > > > > def f(n): > > > if n: > > > f(n-1) > > > > > > f(500_000) > > > > > > The following program will raise a StackOverflow, without causing > > > a > > > VM > > > crash: > > > > > > sys.setrecursionlimit(1_000_000) > > > > > > class X: > > > def __add__(self, other): > > > return self + other > > > > > > X() + 1 > > > > > > > > > This is appreciated! I recently spend quite a bit of time trying to > > solve a StackOverflow like this in NumPy (and was unable to fully > > resolve it). Of course the code triggering it was bordering on > > malicious, but it would be nice if it was clear how to not > > segfault. > > > > Just some questions/notes: > > > > * We currently mostly use `Py_EnterRecursiveCall()` in situations > > where > > we need to safe-guard against "almost python" recursions. For > > example > > an attribute lookup that returns `self`, or a list containing > > itself. > > In those cases the python recursion limit seems a bit nicer (lower > > and > > easier to understand). > > I am not sure it actually matters much, but my question is: Are we > > sure > > we want to replace all (or even many) C recursion checks? > > Would it help if you had the ability to increase and decrease the > recursion depth, as `Py_EnterRecursiveCall()` currently does? > > I'm reluctant to expose it, as it might encourage C code authors to > use > it, rather than `Py_CheckStackDepth()` resulting in crashes. > > To be robust, C code must make a call to `Py_CheckStackDepth()`. > To check the recursion limit as well would be extra overhead. > > > > > * Assuming we swap `Py_EnterRecursiveCall()` logic, I am wondering > > if a > > new `StackOverflow` exception name is useful. It may create two > > names > > for almost identical Python code: If you unpack a list containing > > itself compared to a mapping implementing `__getitem__` in Python > > you > > would get different exceptions. > > True, but they are different. One is a soft limit that can be > increased, > the other is a hard limit that cannot (at least not easily).
Right. I think my confusion completely resolves around your proposed
change of `Py_EnterRecursiveCall()`.
A simple example (written in C):
def depth(obj, current=0):
Py_EnterRecursiveCall()
if isinstance(obj, sequence): # has the sequence slots
return depth(obj[0], current+1)
return current
will never hit the "depth" limit for a self containing list or even
sequence (as long as `GetItem` can use the C-level slot).
But `obj[0]` could nevertheless return a non-trivial object (one with
`__del__`, definitely a container with unrelated objects that could use
deleting).
As the author of the function, I have no knowledge over how much stack
space cleaning those up may require?
And say someone adds a check for `Py_CheckStackDepth()` inside a
dealloc, then this might have to cause a fatal error?
Maybe it should even be a fatal error by default in some cases?
Also, if the code is slow, the previous recursion may guard against
hanging (arguably, if that is the case I probably add an interrupt
check, I admit).
Long story short, I will trust you guys on it of course, but I am not
yet convinced that replacing the check will actually do any good (as
opposed to adding and/or providing the additional check) or even be a
service to users (since I assume that the vast majority do not crank up
the recursion limit to huge values).
Cheers,
Sebastian
>
> >
> > * `Py_CheckStackDepthWithHeadRoom()` is usually not necessary,
> > because
> > `Py_CheckStackDepth()` would leave plenty of headroom for typical
> > clean-up?
>
> What is "typical" clean up? I would hope that typical cleanup is to
> return immediately.
>
> > Can we assume that DECREF's (i.e. list, tuple), will never check
> > the
> > depth, so head-room is usually not necessary? This is all good,
> > but I
> > am not immediately sure when `Py_CheckStackDepthWithHeadRoom()`
> > would
> > be necessary (There are probably many cases where it clearly is,
> > but is
> > it ever for fairly simple code?).
>
> Ideally, Dealloc should call `Py_CheckStackDepth()`, but it will need
> to be very cheap for that to be practical.
>
> If C code is consuming the stack, its responsibility is to not
> overflow.
> We can't make you call `Py_CheckStackDepth()`, but we can provide it,
> so
> you that will have no excuse for blowing the stack :)
>
> > What happens if the maximum stack depth is reached while a
> > `StackOverflow` exception is already set? Will the current
> > "watermark"
> > mechanism remain, or could there be a simple rule that an uncleared
> > `StackOverflow` exception ensures some additional head-room?
>
> When an exception is "set", the C code should be unwinding stack,
> so those states shouldn't be possible.
>
> We can't give you extra headroom. The C stack is a fixed size.
> That's why `Py_CheckStackDepthWithHeadRoom()` is provided, if
> `Py_CheckStackDepth()` fails then it is too late to do much.
>
> Cheers,
> Mark.
>
> >
> > Cheers,
> >
> > Sebastian
> >
> >
> >
> > > -----------
> > >
> > > The full PEP can be found here:
> > > https://www.python.org/dev/peps/pep-0651
> > >
> > > As always, comments are welcome.
> > >
> > > Cheers,
> > > Mark.
> > > _______________________________________________
> > > Python-Dev mailing list -- python-dev@python.org
> > > To unsubscribe send an email to python-dev-le...@python.org
> > > https://mail.python.org/mailman3/lists/python-dev.python.org/
> > > Message archived at
> > > https://mail.python.org/archives/list/python-dev@python.org/message/ZY32N43YZJM3WYXSVD7OCGVNDGPR6DUM/
> > > Code of Conduct: http://python.org/psf/codeofconduct/
> > >
> >
> >
> > _______________________________________________
> > Python-Dev mailing list -- python-dev@python.org
> > To unsubscribe send an email to python-dev-le...@python.org
> > https://mail.python.org/mailman3/lists/python-dev.python.org/
> > Message archived at
> > https://mail.python.org/archives/list/python-dev@python.org/message/N456CVKWZ3E3VKPOE2DZMFLVSMOK5BSF/
> > Code of Conduct: http://python.org/psf/codeofconduct/
> >
> _______________________________________________
> Python-Dev mailing list -- python-dev@python.org
> To unsubscribe send an email to python-dev-le...@python.org
> https://mail.python.org/mailman3/lists/python-dev.python.org/
> Message archived at
> https://mail.python.org/archives/list/python-dev@python.org/message/GSR6DCTXK4WWP2U5X65A3HBVY4UWQUPR/
> Code of Conduct: http://python.org/psf/codeofconduct/
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/QJICZO4IZ6EYLQS4WVPU7ON6DOO4IXN6/ Code of Conduct: http://python.org/psf/codeofconduct/
